Merge pull request #1130 from Growstuff/dev

Release 17

.codeclimate.yml

@@ -38,3 +38,4 @@ exclude_paths:
 - db/
 - spec/
 - public/
+- app/assets/stylesheets/bootstrap-accessibility.css

.rubocop.yml

@@ -8,6 +8,9 @@ AllCops:
     - 'db/schema.rb'
     - 'vendor/**/*'
 
+Rails:
+  Enabled: true
+
 Style/FileName:
   Exclude:
     - 'Gemfile'
@@ -34,7 +37,7 @@ Metrics/MethodLength:
 # Remove the following once the code style matches
 # Offense count: 59
 Metrics/AbcSize:
-  Max: 115
+  Max: 38
 
 # Offense count: 5
 # Configuration parameters: CountComments.
@@ -66,3 +69,10 @@ Style/Documentation:
 
 Style/FrozenStringLiteralComment:
   Enabled: false
+
+# Configuration parameters: Include.
+# Include: app/**/*.rb, config/**/*.rb, db/**/*.rb, lib/**/*.rb
+Rails/Output:
+  Exclude:
+    - 'config/unicorn.rb'
+    - 'db/seeds.rb'

.rubocop_todo.yml

@@ -6,11 +6,6 @@
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 2
-Lint/AmbiguousOperator:
-  Exclude:
-    - 'spec/factories/member.rb'
-
 # Offense count: 24
 Lint/AmbiguousRegexpLiteral:
   Exclude:
@@ -26,26 +21,11 @@ Lint/AmbiguousRegexpLiteral:
     - 'spec/views/members/show.rss.haml_spec.rb'
     - 'spec/views/posts/show.html.haml_spec.rb'
 
-# Offense count: 1
-# Configuration parameters: AllowSafeAssignment.
-Lint/AssignmentInCondition:
-  Exclude:
-    - 'app/models/member.rb'
-
 # Offense count: 1
 Lint/HandleExceptions:
   Exclude:
     - 'lib/tasks/testing.rake'
 
-# Offense count: 8
-Lint/ParenthesesAsGroupedExpression:
-  Exclude:
-    - 'app/mailers/notifier.rb'
-    - 'spec/features/harvests/browse_harvests_spec.rb'
-    - 'spec/models/follow_spec.rb'
-    - 'spec/models/member_spec.rb'
-    - 'spec/views/plantings/show.html.haml_spec.rb'
-
 # Offense count: 15
 # Cop supports --auto-correct.
 # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments.
@@ -69,28 +49,6 @@ Lint/UnusedMethodArgument:
     - 'app/validators/approved_validator.rb'
     - 'spec/views/plantings/show.html.haml_spec.rb'
 
-# Offense count: 52
-Lint/UselessAssignment:
-  Exclude:
-    - 'app/controllers/crops_controller.rb'
-    - 'app/models/crop.rb'
-    - 'app/models/member.rb'
-    - 'config.rb'
-    - 'config/compass.rb'
-    - 'config/setup_load_paths.rb'
-    - 'db/seeds.rb'
-    - 'lib/tasks/growstuff.rake'
-    - 'spec/controllers/admin/orders_controller_spec.rb'
-    - 'spec/features/signin_spec.rb'
-    - 'spec/features/unsubscribing_spec.rb'
-    - 'spec/helpers/gardens_helper_spec.rb'
-    - 'spec/helpers/notifications_helper_spec.rb'
-    - 'spec/helpers/seeds_helper_spec.rb'
-    - 'spec/models/crop_spec.rb'
-    - 'spec/models/garden_spec.rb'
-    - 'spec/models/member_spec.rb'
-    - 'spec/support/controller_macros.rb'
-
 # Offense count: 5
 Lint/Void:
   Exclude:
@@ -108,33 +66,6 @@ Performance/StringReplacement:
     - 'app/models/seed.rb'
     - 'spec/rails_helper.rb'
 
-# Offense count: 21
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles, Include.
-# SupportedStyles: action, filter
-# Include: app/controllers/**/*.rb
-Rails/ActionFilter:
-  Exclude:
-    - 'app/controllers/account_types_controller.rb'
-    - 'app/controllers/accounts_controller.rb'
-    - 'app/controllers/alternate_names_controller.rb'
-    - 'app/controllers/application_controller.rb'
-    - 'app/controllers/authentications_controller.rb'
-    - 'app/controllers/comments_controller.rb'
-    - 'app/controllers/crops_controller.rb'
-    - 'app/controllers/follows_controller.rb'
-    - 'app/controllers/gardens_controller.rb'
-    - 'app/controllers/harvests_controller.rb'
-    - 'app/controllers/notifications_controller.rb'
-    - 'app/controllers/order_items_controller.rb'
-    - 'app/controllers/orders_controller.rb'
-    - 'app/controllers/plantings_controller.rb'
-    - 'app/controllers/posts_controller.rb'
-    - 'app/controllers/products_controller.rb'
-    - 'app/controllers/roles_controller.rb'
-    - 'app/controllers/scientific_names_controller.rb'
-    - 'app/controllers/seeds_controller.rb'
-
 # Offense count: 10
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: strict, flexible
@@ -149,39 +80,6 @@ Rails/Date:
     - 'spec/features/plantings/planting_a_crop_spec.rb'
     - 'spec/features/shared_examples/append_date.rb'
 
-# Offense count: 42
-# Cop supports --auto-correct.
-# Configuration parameters: Whitelist.
-# Whitelist: find_by_sql
-Rails/DynamicFindBy:
-  Exclude:
-    - 'app/controllers/alternate_names_controller.rb'
-    - 'app/controllers/comments_controller.rb'
-    - 'app/controllers/gardens_controller.rb'
-    - 'app/controllers/harvests_controller.rb'
-    - 'app/controllers/notifications_controller.rb'
-    - 'app/controllers/plantings_controller.rb'
-    - 'app/controllers/posts_controller.rb'
-    - 'app/controllers/scientific_names_controller.rb'
-    - 'app/controllers/seeds_controller.rb'
-    - 'app/models/crop.rb'
-    - 'app/models/member.rb'
-    - 'app/models/order.rb'
-    - 'db/seeds.rb'
-    - 'lib/tasks/growstuff.rake'
-    - 'spec/controllers/roles_controller_spec.rb'
-    - 'spec/models/crop_spec.rb'
-    - 'spec/models/scientific_name_spec.rb'
-
-# Offense count: 7
-# Cop supports --auto-correct.
-# Configuration parameters: Include.
-# Include: app/models/**/*.rb
-Rails/FindBy:
-  Exclude:
-    - 'app/models/member.rb'
-    - 'app/models/post.rb'
-
 # Offense count: 11
 # Configuration parameters: Include.
 # Include: app/models/**/*.rb
@@ -197,36 +95,6 @@ Rails/HasAndBelongsToMany:
     - 'app/models/product.rb'
     - 'app/models/role.rb'
 
-# Offense count: 89
-# Cop supports --auto-correct.
-# Configuration parameters: Include.
-# Include: spec/**/*, test/**/*
-Rails/HttpPositionalArguments:
-  Exclude:
-    - 'spec/controllers/admin/orders_controller_spec.rb'
-    - 'spec/controllers/comments_controller_spec.rb'
-    - 'spec/controllers/crops_controller_spec.rb'
-    - 'spec/controllers/harvests_controller_spec.rb'
-    - 'spec/controllers/member_controller_spec.rb'
-    - 'spec/controllers/notifications_controller_spec.rb'
-    - 'spec/controllers/order_items_controller_spec.rb'
-    - 'spec/controllers/orders_controller_spec.rb'
-    - 'spec/controllers/places_controller_spec.rb'
-    - 'spec/controllers/plantings_controller_spec.rb'
-    - 'spec/controllers/posts_controller_spec.rb'
-    - 'spec/controllers/roles_controller_spec.rb'
-    - 'spec/controllers/scientific_names_controller_spec.rb'
-    - 'spec/controllers/seeds_controller_spec.rb'
-    - 'spec/controllers/shop_controller_spec.rb'
-
-# Offense count: 15
-# Configuration parameters: Include.
-# Include: app/**/*.rb, config/**/*.rb, db/**/*.rb, lib/**/*.rb
-Rails/Output:
-  Exclude:
-    - 'config/unicorn.rb'
-    - 'db/seeds.rb'
-
 # Offense count: 3
 Rails/OutputSafety:
   Exclude:
@@ -234,21 +102,6 @@ Rails/OutputSafety:
     - 'app/helpers/auto_suggest_helper.rb'
     - 'app/helpers/gardens_helper.rb'
 
-# Offense count: 2
-# Cop supports --auto-correct.
-Rails/PluralizationGrammar:
-  Exclude:
-    - 'spec/features/plantings/planting_a_crop_spec.rb'
-    - 'spec/models/member_spec.rb'
-
-# Offense count: 1
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: referer, referrer
-Rails/RequestReferer:
-  Exclude:
-    - 'app/controllers/application_controller.rb'
-
 # Offense count: 9
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: strict, flexible
@@ -402,188 +255,6 @@ Style/DefWithParentheses:
   Exclude:
     - 'spec/views/posts/_single.html.haml_spec.rb'
 
-# Offense count: 178
-Style/Documentation:
-  Exclude:
-    - 'spec/**/*'
-    - 'test/**/*'
-    - 'app/controllers/account_types_controller.rb'
-    - 'app/controllers/accounts_controller.rb'
-    - 'app/controllers/admin/orders_controller.rb'
-    - 'app/controllers/admin_controller.rb'
-    - 'app/controllers/alternate_names_controller.rb'
-    - 'app/controllers/application_controller.rb'
-    - 'app/controllers/authentications_controller.rb'
-    - 'app/controllers/comments_controller.rb'
-    - 'app/controllers/crops_controller.rb'
-    - 'app/controllers/follows_controller.rb'
-    - 'app/controllers/forums_controller.rb'
-    - 'app/controllers/gardens_controller.rb'
-    - 'app/controllers/harvests_controller.rb'
-    - 'app/controllers/home_controller.rb'
-    - 'app/controllers/members_controller.rb'
-    - 'app/controllers/notifications_controller.rb'
-    - 'app/controllers/order_items_controller.rb'
-    - 'app/controllers/orders_controller.rb'
-    - 'app/controllers/pages_controller.rb'
-    - 'app/controllers/passwords_controller.rb'
-    - 'app/controllers/photos_controller.rb'
-    - 'app/controllers/places_controller.rb'
-    - 'app/controllers/plant_parts_controller.rb'
-    - 'app/controllers/plantings_controller.rb'
-    - 'app/controllers/posts_controller.rb'
-    - 'app/controllers/products_controller.rb'
-    - 'app/controllers/registrations_controller.rb'
-    - 'app/controllers/robots_controller.rb'
-    - 'app/controllers/roles_controller.rb'
-    - 'app/controllers/scientific_names_controller.rb'
-    - 'app/controllers/seeds_controller.rb'
-    - 'app/controllers/sessions_controller.rb'
-    - 'app/controllers/shop_controller.rb'
-    - 'app/helpers/application_helper.rb'
-    - 'app/helpers/auto_suggest_helper.rb'
-    - 'app/helpers/crops_helper.rb'
-    - 'app/helpers/gardens_helper.rb'
-    - 'app/helpers/harvests_helper.rb'
-    - 'app/helpers/notifications_helper.rb'
-    - 'app/helpers/plantings_helper.rb'
-    - 'app/helpers/seeds_helper.rb'
-    - 'app/mailers/notifier.rb'
-    - 'app/models/ability.rb'
-    - 'app/models/account.rb'
-    - 'app/models/account_type.rb'
-    - 'app/models/alternate_name.rb'
-    - 'app/models/authentication.rb'
-    - 'app/models/comment.rb'
-    - 'app/models/crop.rb'
-    - 'app/models/follow.rb'
-    - 'app/models/forum.rb'
-    - 'app/models/garden.rb'
-    - 'app/models/harvest.rb'
-    - 'app/models/member.rb'
-    - 'app/models/notification.rb'
-    - 'app/models/order.rb'
-    - 'app/models/order_item.rb'
-    - 'app/models/photo.rb'
-    - 'app/models/plant_part.rb'
-    - 'app/models/planting.rb'
-    - 'app/models/post.rb'
-    - 'app/models/product.rb'
-    - 'app/models/role.rb'
-    - 'app/models/scientific_name.rb'
-    - 'app/models/seed.rb'
-    - 'app/validators/approved_validator.rb'
-    - 'config/application.rb'
-    - 'config/initializers/comfortable_mexican_sofa.rb'
-    - 'db/migrate/20120903092956_devise_create_users.rb'
-    - 'db/migrate/20120903112806_add_username_to_users.rb'
-    - 'db/migrate/20121001212604_create_crops.rb'
-    - 'db/migrate/20121003190731_require_system_name_for_crops.rb'
-    - 'db/migrate/20121027035231_add_slug_to_crops.rb'
-    - 'db/migrate/20121105032913_create_gardens.rb'
-    - 'db/migrate/20121106101718_add_slug_to_users.rb'
-    - 'db/migrate/20121107012827_create_scientific_names.rb'
-    - 'db/migrate/20121108105440_create_updates.rb'
-    - 'db/migrate/20121109130033_add_creation_index_to_updates.rb'
-    - 'db/migrate/20121203034745_add_tos_agreement_to_users.rb'
-    - 'db/migrate/20121214224227_add_slug_to_updates.rb'
-    - 'db/migrate/20121219022554_create_plantings.rb'
-    - 'db/migrate/20130113045802_rename_updates_to_posts.rb'
-    - 'db/migrate/20130113060852_rename_users_to_members.rb'
-    - 'db/migrate/20130113081521_rename_post_member_to_author.rb'
-    - 'db/migrate/20130113095802_rename_garden_member_to_owner.rb'
-    - 'db/migrate/20130118031942_add_description_to_gardens.rb'
-    - 'db/migrate/20130118043431_add_slug_to_plantings.rb'
-    - 'db/migrate/20130206033956_create_comments.rb'
-    - 'db/migrate/20130206051328_add_show_email_to_member.rb'
-    - 'db/migrate/20130208034248_require_fields_for_comments.rb'
-    - 'db/migrate/20130212001748_add_geo_to_members.rb'
-    - 'db/migrate/20130212123628_create_notifications.rb'
-    - 'db/migrate/20130213014511_create_forums.rb'
-    - 'db/migrate/20130213015708_add_forum_to_posts.rb'
-    - 'db/migrate/20130214024117_create_roles.rb'
-    - 'db/migrate/20130214034838_add_members_roles_table.rb'
-    - 'db/migrate/20130215131921_rename_notification_fields.rb'
-    - 'db/migrate/20130220044605_add_slug_to_forums.rb'
-    - 'db/migrate/20130220044642_add_slug_to_roles.rb'
-    - 'db/migrate/20130222060730_default_read_to_false.rb'
-    - 'db/migrate/20130326092227_change_planted_at_to_date.rb'
-    - 'db/migrate/20130327120024_add_send_email_to_member.rb'
-    - 'db/migrate/20130329045744_add_sunniness_to_planting.rb'
-    - 'db/migrate/20130404174459_create_authentications.rb'
-    - 'db/migrate/20130409103549_make_post_subject_non_null.rb'
-    - 'db/migrate/20130409162140_add_name_to_authentications.rb'
-    - 'db/migrate/20130507105357_create_products.rb'
-    - 'db/migrate/20130507110411_create_orders.rb'
-    - 'db/migrate/20130507113915_add_orders_products_table.rb'
-    - 'db/migrate/20130508050711_add_completed_to_order.rb'
-    - 'db/migrate/20130508104506_create_photos.rb'
-    - 'db/migrate/20130509123711_add_metadata_to_photos.rb'
-    - 'db/migrate/20130514124515_add_parent_to_crop.rb'
-    - 'db/migrate/20130515033842_create_order_items.rb'
-    - 'db/migrate/20130515054017_change_order_member_id_to_integer.rb'
-    - 'db/migrate/20130515122301_change_prices_to_integers.rb'
-    - 'db/migrate/20130517015920_create_account_details.rb'
-    - 'db/migrate/20130517051922_create_account_types.rb'
-    - 'db/migrate/20130517234458_require_account_type_name.rb'
-    - 'db/migrate/20130518000339_add_columns_to_product.rb'
-    - 'db/migrate/20130518002942_rename_account_detail_to_account.rb'
-    - 'db/migrate/20130529032813_add_express_token_to_orders.rb'
-    - 'db/migrate/20130531110729_add_photos_plantings_table.rb'
-    - 'db/migrate/20130601011725_change_flickr_photo_id_to_string.rb'
-    - 'db/migrate/20130606230333_change_product_description_to_text.rb'
-    - 'db/migrate/20130606233733_add_recommended_price_to_product.rb'
-    - 'db/migrate/20130705104238_add_planted_from_to_planting.rb'
-    - 'db/migrate/20130715110134_create_seeds.rb'
-    - 'db/migrate/20130718005600_change_use_by_to_plant_before_on_seed.rb'
-    - 'db/migrate/20130718011247_add_trading_to_seeds.rb'
-    - 'db/migrate/20130722050836_remove_tradable_from_seeds.rb'
-    - 'db/migrate/20130723103128_set_default_tradable_to_on_seed.rb'
-    - 'db/migrate/20130723110702_add_slug_to_seed.rb'
-    - 'db/migrate/20130809012511_add_bio_to_members.rb'
-    - 'db/migrate/20130819004549_add_planting_count_to_crop.rb'
-    - 'db/migrate/20130821011352_add_creator_to_crops.rb'
-    - 'db/migrate/20130821073736_add_creator_to_scientific_name.rb'
-    - 'db/migrate/20130826012139_add_owner_to_planting.rb'
-    - 'db/migrate/20130826023159_add_plantings_count_to_member.rb'
-    - 'db/migrate/20130827105823_add_newsletter_to_member.rb'
-    - 'db/migrate/20130913015118_add_referral_code_to_order.rb'
-    - 'db/migrate/20130917053547_create_harvests.rb'
-    - 'db/migrate/20130917060257_change_harvest_notes_to_description.rb'
-    - 'db/migrate/20130917071545_change_harvest_units_to_unit.rb'
-    - 'db/migrate/20130917075803_add_slug_to_harvests.rb'
-    - 'db/migrate/20130925050304_add_weight_to_harvests.rb'
-    - 'db/migrate/20131018101204_rename_system_name_to_name.rb'
-    - 'db/migrate/20131025104228_add_fields_to_gardens.rb'
-    - 'db/migrate/20131029053113_add_plant_part_to_harvests.rb'
-    - 'db/migrate/20131030230908_create_plant_parts.rb'
-    - 'db/migrate/20131030231202_change_plant_part_to_plant_part_id.rb'
-    - 'db/migrate/20131031000655_add_slug_to_plant_part.rb'
-    - 'db/migrate/20140718075753_default_plantings_count_to_zero.rb'
-    - 'db/migrate/20140829230600_add_finished_to_planting.rb'
-    - 'db/migrate/20140905001730_add_harvests_photos_table.rb'
-    - 'db/migrate/20140928044231_add_crops_posts_table.rb'
-    - 'db/migrate/20140928085713_add_send_planting_reminder_to_member.rb'
-    - 'db/migrate/20141002022459_create_index_harvest_photos.rb'
-    - 'db/migrate/20141018111015_create_alternate_names.rb'
-    - 'db/migrate/20141111130849_create_follows.rb'
-    - 'db/migrate/20141119130555_change_follows_member_id_to_follower_id.rb'
-    - 'db/migrate/20150124110540_add_properties_to_seeds.rb'
-    - 'db/migrate/20150127043022_add_gardens_photos_table.rb'
-    - 'db/migrate/20150129034206_add_si_weight_to_harvest.rb'
-    - 'db/migrate/20150130224814_add_requester_to_crops.rb'
-    - 'db/migrate/20150201052245_create_cms.rb'
-    - 'db/migrate/20150201053200_add_approval_status_to_crops.rb'
-    - 'db/migrate/20150201062506_add_reason_for_rejection_to_crops.rb'
-    - 'db/migrate/20150201064502_add_request_notes_to_crops.rb'
-    - 'db/migrate/20150209105410_add_rejection_notes_to_crops.rb'
-    - 'db/migrate/20150625224805_add_days_before_maturity_to_plantings.rb'
-    - 'db/migrate/20150824145414_add_member_preferred_image.rb'
-    - 'lib/actions/oauth_signup_action.rb'
-    - 'lib/geocodable.rb'
-    - 'lib/haml/filters/escaped_markdown.rb'
-    - 'lib/haml/filters/growstuff_markdown.rb'
-
 # Offense count: 10
 # Cop supports --auto-correct.
 Style/EachForSimpleLoop:
@@ -613,31 +284,6 @@ Style/FormatString:
     - 'spec/helpers/application_helper_spec.rb'
     - 'spec/views/shop/index_spec.rb'
 
-# Offense count: 35
-# Configuration parameters: MinBodyLength.
-Style/GuardClause:
-  Exclude:
-    - 'app/controllers/members_controller.rb'
-    - 'app/helpers/harvests_helper.rb'
-    - 'app/helpers/plantings_helper.rb'
-    - 'app/mailers/notifier.rb'
-    - 'app/models/ability.rb'
-    - 'app/models/account.rb'
-    - 'app/models/crop.rb'
-    - 'app/models/garden.rb'
-    - 'app/models/harvest.rb'
-    - 'app/models/member.rb'
-    - 'app/models/notification.rb'
-    - 'app/models/order.rb'
-    - 'app/models/order_item.rb'
-    - 'app/models/photo.rb'
-    - 'app/models/seed.rb'
-    - 'app/validators/approved_validator.rb'
-    - 'config/initializers/comfortable_mexican_sofa.rb'
-    - 'lib/geocodable.rb'
-    - 'spec/lib/haml/filters/growstuff_markdown_spec.rb'
-    - 'spec/support/elasticsearch_helpers.rb'
-
 # Offense count: 4
 Style/IdenticalConditionalBranches:
   Exclude:
@@ -921,32 +567,6 @@ Style/RedundantParentheses:
     - 'app/helpers/plantings_helper.rb'
     - 'app/models/garden.rb'
 
-# Offense count: 62
-# Cop supports --auto-correct.
-# Configuration parameters: AllowMultipleReturnValues.
-Style/RedundantReturn:
-  Exclude:
-    - 'app/helpers/application_helper.rb'
-    - 'app/helpers/harvests_helper.rb'
-    - 'app/helpers/plantings_helper.rb'
-    - 'app/mailers/notifier.rb'
-    - 'app/models/account.rb'
-    - 'app/models/crop.rb'
-    - 'app/models/forum.rb'
-    - 'app/models/garden.rb'
-    - 'app/models/harvest.rb'
-    - 'app/models/member.rb'
-    - 'app/models/order.rb'
-    - 'app/models/photo.rb'
-    - 'app/models/plant_part.rb'
-    - 'app/models/planting.rb'
-    - 'app/models/seed.rb'
-    - 'lib/haml/filters/escaped_markdown.rb'
-    - 'lib/haml/filters/growstuff_markdown.rb'
-    - 'spec/controllers/accounts_controller_spec.rb'
-    - 'spec/lib/haml/filters/growstuff_markdown_spec.rb'
-    - 'spec/views/devise/shared/_links_spec.rb'
-
 # Offense count: 56
 # Cop supports --auto-correct.
 Style/RedundantSelf:

.ruby-version

@@ -1 +1 @@
-2.3.1
+2.3.3

.travis.yml

@@ -11,12 +11,12 @@ env:
   global:
     secure: "Z5TpM2jEX4UCvNePnk/LwltQX48U2u9BRc+Iypr1x9QW2o228QJhPIOH39a8RMUrepGnkQIq9q3ZRUn98RfrJz1yThtlNFL3NmzdQ57gKgjGwfpa0e4Dwj/ZJqV2D84tDGjvdVYLP7zzaYZxQcwk/cgNpzKf/jq97HLNP7CYuf4="
 rvm:
-  - 2.3.1
+  - 2.3.3
 before_install:
   - export PATH=$PWD/travis_phantomjs/phantomjs-2.1.1-linux-x86_64/bin:$PATH
   - >
     if [ $(phantomjs --version) != '2.1.1' ]; then
-    PHANTOM_URL=https://bitbucket.org/ariya/phantomjs/downloads/phantomjs-2.1.1-linux-x86_64.tar.bz2;
+    PHANTOM_URL=https://assets.membergetmember.co/software/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;

CONTRIBUTING.md

@@ -3,7 +3,7 @@ Thanks for contributing to Growstuff!
 When you create a pull request, please include the following:
 
 * Mention the issue it solves (eg. #123)
-* Your code should follow our [Coding style guide](http://wiki.growstuff.org/index.php/Coding_style_guide)
+* Your code should follow our [Coding style guide](https://github.com/Growstuff/growstuff/wiki/Development-process-overview#coding-practices)
 * Make sure you have automated tests for your work, where possible.
 * Add your name (and that of your pair partner, if any) to [CONTRIBUTORS.md](CONTRIBUTORS.md).
 

CONTRIBUTORS.md

@@ -72,3 +72,4 @@ submit the change with your pull request.
 - Charley Lewittes / [ctlewitt](https://github.com/ctlewitt)
 - Kristine Nicole Polvoriza / [polveenomials](https://github.com/polveenomials)
 - Brenda Wallace / [br3nda](https://github.com/br3nda)
+- Jim Stallings / [jestallin](https://github.com/jestallin)

Gemfile

@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 source 'https://rubygems.org'
 
-ruby '2.3.1'
+ruby '2.3.3'
 
-gem 'rails', '~> 4.2.1'
+gem 'rails', '~> 4.2.7'
 
 gem 'bundler', '>=1.1.5'
 
@@ -71,6 +71,11 @@ gem 'omniauth-facebook'
 # client for Elasticsearch. Elasticsearch is a flexible
 # and powerful, distributed, real-time search and analytics engine.
 # An example of the use in the project is fuzzy crop search.
+
+# Project does not use semver, so we want to be in sync with the version of 
+# elasticsearch we use
+# See https://github.com/elastic/elasticsearch-ruby#compatibility
+gem "elasticsearch-api", "~> 2.0.0"
 gem "elasticsearch-model"
 gem "elasticsearch-rails"
 
@@ -113,6 +118,7 @@ group :development, :test do
   gem 'poltergeist'                     # for headless JS testing
   gem 'i18n-tasks'                      # adds tests for finding missing and unused translations
   gem 'selenium-webdriver'
+  gem 'haml-i18n-extractor'
   gem "active_merchant-paypal-bogus-gateway"
   gem 'rubocop', require: false
 end

Gemfile.lock

@@ -52,7 +52,7 @@ GEM
       public_suffix (~> 2.0, >= 2.0.2)
     arel (6.0.3)
     ast (2.3.0)
-    autoprefixer-rails (6.4.0.2)
+    autoprefixer-rails (6.5.3.1)
       execjs
     bcrypt (3.1.11)
     better_errors (2.1.1)
@@ -95,8 +95,8 @@ GEM
     cliver (0.3.2)
     cocaine (0.5.8)
       climate_control (>= 0.0.3, < 1.0)
-    codeclimate-test-reporter (0.6.0)
-      simplecov (>= 0.7.1, < 1.0.0)
+    codeclimate-test-reporter (1.0.3)
+      simplecov
     codemirror-rails (5.16.0)
       railties (>= 3.0, < 6.0)
     coderay (1.1.1)
@@ -106,7 +106,7 @@ GEM
     coffee-script (2.4.1)
       coffee-script-source
       execjs
-    coffee-script-source (1.10.0)
+    coffee-script-source (1.11.1)
     comfortable_mexican_sofa (1.12.9)
       active_link_to (>= 1.0.0)
       bootstrap-sass (>= 3.2.0)
@@ -204,13 +204,19 @@ GEM
       rspec (>= 2.99.0, < 4.0)
     haml (4.0.7)
       tilt
+    haml-i18n-extractor (0.5.9)
+      activesupport
+      haml
+      highline
+      tilt
+      trollop (= 1.16.2)
     haml-rails (0.9.0)
       actionpack (>= 4.0.1)
       activesupport (>= 4.0.1)
       haml (>= 4.0.6, < 5.0)
       html2haml (>= 1.0.1)
       railties (>= 4.0.1)
-    hashie (3.4.4)
+    hashie (3.4.6)
     heroku-api (0.4.2)
       excon (~> 0.45)
       multi_json (~> 1.8)
@@ -272,9 +278,9 @@ GEM
     mime-types-data (3.2016.0521)
     mimemagic (0.3.2)
     mini_portile2 (2.1.0)
-    minitest (5.9.1)
+    minitest (5.10.1)
     multi_json (1.11.3)
-    multi_xml (0.5.5)
+    multi_xml (0.6.0)
     multipart-post (2.0.0)
     nenv (0.3.0)
     newrelic_rpm (3.17.1.326)
@@ -293,7 +299,7 @@ GEM
     omniauth (1.3.1)
       hashie (>= 1.2, < 4)
       rack (>= 1.0, < 3)
-    omniauth-facebook (3.0.0)
+    omniauth-facebook (4.0.0)
       omniauth-oauth2 (~> 1.2)
     omniauth-flickr (0.0.19)
       multi_json (~> 1.11.0)
@@ -370,7 +376,7 @@ GEM
       thor (>= 0.18.1, < 2.0)
     rainbow (2.1.0)
     raindrops (0.17.0)
-    rake (11.3.0)
+    rake (12.0.0)
     rb-fsevent (0.9.8)
     rb-inotify (0.9.7)
       ffi (>= 0.5.0)
@@ -455,11 +461,12 @@ GEM
       tins (~> 1.0)
     terminal-table (1.7.3)
       unicode-display_width (~> 1.1.1)
-    thor (0.19.3)
+    thor (0.19.4)
     thread (0.2.2)
     thread_safe (0.3.5)
     tilt (2.0.5)
     tins (1.13.0)
+    trollop (1.16.2)
     tzinfo (1.2.2)
       thread_safe (~> 0.1)
     uglifier (2.7.2)
@@ -479,7 +486,7 @@ GEM
     websocket-driver (0.6.4)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.2)
-    will_paginate (3.1.0)
+    will_paginate (3.1.5)
     xpath (2.0.0)
       nokogiri (~> 1.3)
 
@@ -511,6 +518,7 @@ DEPENDENCIES
   dalli
   database_cleaner (~> 1.5.0)
   devise (>= 4.0.0)
+  elasticsearch-api (~> 2.0.0)
   elasticsearch-model
   elasticsearch-rails
   factory_girl_rails
@@ -524,6 +532,7 @@ DEPENDENCIES
   guard
   guard-rspec
   haml
+  haml-i18n-extractor
   haml-rails
   heroku-api
   i18n-tasks
@@ -544,7 +553,7 @@ DEPENDENCIES
   poltergeist
   pry
   quiet_assets
-  rails (~> 4.2.1)
+  rails (~> 4.2.7)
   rails_12factor
   rake (>= 10.0.0)
   rspec-activemodel-mocks
@@ -561,7 +570,7 @@ DEPENDENCIES
   will_paginate (~> 3.0)
 
 RUBY VERSION
-   ruby 2.3.1p112
+   ruby 2.3.3p222
 
 BUNDLED WITH
    1.13.6

app/constants/photo_models.rb

@@ -0,0 +1,38 @@
+module Growstuff
+  module Constants
+    class PhotoModels
+      PLANTING = { type: 'planting', class: 'Planting', relation: 'plantings' }.freeze
+      HARVEST = { type: 'harvest', class: 'Harvest', relation: 'harvests' }.freeze
+      GARDEN = { type: 'garden', class: 'Garden', relation: 'gardens' }.freeze
+      SEED = { type: 'seed', class: 'Seed', relation: 'seeds' }.freeze
+
+      ALL = [PLANTING, HARVEST, GARDEN, SEED].freeze
+
+      def self.types
+        ALL.map do |model|
+          model[:type]
+        end
+      end
+
+      def self.relations
+        ALL.map do |model|
+          model[:relation]
+        end
+      end
+
+      def self.get_relation(object, type)
+        relation = ALL.select do |model|
+          model[:type] == type
+        end[0][:relation]
+        object.send(relation)
+      end
+
+      def self.get_item(type)
+        class_name = ALL.select do |model|
+          model[:type] == type
+        end[0][:class]
+        class_name.constantize
+      end
+    end
+  end
+end

app/controllers/account_types_controller.rb

@@ -1,5 +1,5 @@
 class AccountTypesController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # GET /account_types
@@ -13,8 +13,6 @@ class AccountTypesController < ApplicationController
 
   # GET /account_types/1
   def show
-    @account_type = AccountType.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
     end
@@ -31,7 +29,6 @@ class AccountTypesController < ApplicationController
 
   # GET /account_types/1/edit
   def edit
-    @account_type = AccountType.find(params[:id])
   end
 
   # POST /account_types
@@ -49,8 +46,6 @@ class AccountTypesController < ApplicationController
 
   # PUT /account_types/1
   def update
-    @account_type = AccountType.find(params[:id])
-
     respond_to do |format|
       if @account_type.update(account_type_params)
         format.html { redirect_to @account_type, notice: 'Account type was successfully updated.' }
@@ -62,7 +57,6 @@ class AccountTypesController < ApplicationController
 
   # DELETE /account_types/1
   def destroy
-    @account_type = AccountType.find(params[:id])
     @account_type.destroy
 
     respond_to do |format|

app/controllers/accounts_controller.rb

@@ -1,5 +1,5 @@
 class AccountsController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # GET /accounts
@@ -13,8 +13,6 @@ class AccountsController < ApplicationController
 
   # GET /accounts/1
   def show
-    @account = Account.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
     end
@@ -22,13 +20,10 @@ class AccountsController < ApplicationController
 
   # GET /accounts/1/edit
   def edit
-    @account = Account.find(params[:id])
   end
 
   # PUT /accounts/1
   def update
-    @account = Account.find(params[:id])
-
     respond_to do |format|
       if @account.update(params[:account])
         format.html { redirect_to @account, notice: 'Account detail was successfully updated.' }

app/controllers/alternate_names_controller.rb

@@ -1,5 +1,5 @@
 class AlternateNamesController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /alternate_names
@@ -17,7 +17,7 @@ class AlternateNamesController < ApplicationController
   # GET /alternate_names/new.json
   def new
     @alternate_name = AlternateName.new
-    @crop = Crop.find_by_id(params[:crop_id]) || Crop.new
+    @crop = Crop.find_or_initialize_by(id: params[:crop_id])
 
     respond_to do |format|
       format.html # new.html.haml
@@ -27,7 +27,6 @@ class AlternateNamesController < ApplicationController
 
   # GET /alternate_names/1/edit
   def edit
-    @alternate_name = AlternateName.find(params[:id])
   end
 
   # POST /alternate_names
@@ -50,8 +49,6 @@ class AlternateNamesController < ApplicationController
   # PUT /alternate_names/1
   # PUT /alternate_names/1.json
   def update
-    @alternate_name = AlternateName.find(params[:id])
-
     respond_to do |format|
       if @alternate_name.update(alternate_name_params)
         format.html { redirect_to @alternate_name.crop, notice: 'Alternate name was successfully updated.' }
@@ -66,7 +63,6 @@ class AlternateNamesController < ApplicationController
   # DELETE /alternate_names/1
   # DELETE /alternate_names/1.json
   def destroy
-    @alternate_name = AlternateName.find(params[:id])
     @crop = @alternate_name.crop
     @alternate_name.destroy
 

app/controllers/application_controller.rb

@@ -3,8 +3,8 @@ class ApplicationController < ActionController::Base
 
   include ApplicationHelper
 
-  after_filter :store_location
-  before_filter :set_locale
+  after_action :store_location
+  before_action :set_locale
 
   def store_location
     if (request.path != "/members/sign_in" &&
@@ -23,7 +23,7 @@ class ApplicationController < ActionController::Base
   end
 
   def after_sign_out_path_for(resource_or_scope)
-    request.referrer
+    request.referer
   end
 
   # tweak CanCan defaults because we don't have a "current_user" method

app/controllers/authentications_controller.rb

@@ -1,5 +1,5 @@
 class AuthenticationsController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # POST /authentications

app/controllers/comments_controller.rb

@@ -1,5 +1,5 @@
 class CommentsController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /comments
@@ -18,7 +18,7 @@ class CommentsController < ApplicationController
   # GET /comments/new.json
   def new
     @comment = Comment.new
-    @post = Post.find_by_id(params[:post_id])
+    @post = Post.find_by(id: params[:post_id])
 
     if @post
       @comments = @post.comments
@@ -34,7 +34,6 @@ class CommentsController < ApplicationController
 
   # GET /comments/1/edit
   def edit
-    @comment = Comment.find(params[:id])
     @comments = @comment.post.comments
   end
 
@@ -58,8 +57,6 @@ class CommentsController < ApplicationController
   # PUT /comments/1
   # PUT /comments/1.json
   def update
-    @comment = Comment.find(params[:id])
-
     # you should never be able to change the author or post when
     # updating
     params[:comment].delete("post_id")
@@ -79,7 +76,6 @@ class CommentsController < ApplicationController
   # DELETE /comments/1
   # DELETE /comments/1.json
   def destroy
-    @comment = Comment.find(params[:id])
     @post = @comment.post
     @comment.destroy
 

app/controllers/crops_controller.rb

@@ -1,7 +1,7 @@
 require 'will_paginate/array'
 
 class CropsController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :hierarchy, :search, :show]
+  before_action :authenticate_member!, except: [:index, :hierarchy, :search, :show]
   load_and_authorize_resource
   skip_authorize_resource only: [:hierarchy, :search]
 
@@ -9,15 +9,12 @@ class CropsController < ApplicationController
   # GET /crops.json
   def index
     @sort = params[:sort]
-    if @sort == 'alpha'
-      # alphabetical order
-      @crops = Crop.includes(:scientific_names, { plantings: :photos })
-      @paginated_crops = @crops.approved.paginate(page: params[:page])
-    else
-      # default to sorting by popularity
-      @crops = Crop.popular.includes(:scientific_names, { plantings: :photos })
-      @paginated_crops = @crops.approved.paginate(page: params[:page])
-    end
+    @crops = if @sort == 'alpha'
+               Crop.includes(:scientific_names, { plantings: :photos })
+             else
+               popular_crops
+             end
+    @paginated_crops = @crops.approved.paginate(page: params[:page])
 
     respond_to do |format|
       format.html
@@ -113,7 +110,6 @@ class CropsController < ApplicationController
 
   # GET /crops/1/edit
   def edit
-    @crop = Crop.find(params[:id])
     @crop.alternate_names.build if @crop.alternate_names.blank?
     @crop.scientific_names.build if @crop.scientific_names.blank?
   end
@@ -158,8 +154,6 @@ class CropsController < ApplicationController
   # PUT /crops/1
   # PUT /crops/1.json
   def update
-    @crop = Crop.find(params[:id])
-
     previous_status = @crop.approval_status
 
     @crop.creator = current_member if previous_status == "pending"
@@ -187,7 +181,6 @@ class CropsController < ApplicationController
   # DELETE /crops/1
   # DELETE /crops/1.json
   def destroy
-    @crop = Crop.find(params[:id])
     @crop.destroy
 
     respond_to do |format|
@@ -198,6 +191,10 @@ class CropsController < ApplicationController
 
   private
 
+  def popular_crops
+    Crop.popular.includes(:scientific_names, { plantings: :photos })
+  end
+
   def recreate_names(param_name, name_type)
     return unless params[param_name].present?
     destroy_names(name_type)

app/controllers/follows_controller.rb

@@ -1,5 +1,5 @@
 class FollowsController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
   skip_load_resource only: :create
 

app/controllers/gardens_controller.rb

@@ -1,15 +1,16 @@
 class GardensController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /gardens
   # GET /gardens.json
   def index
-    @gardens = Garden.paginate(page: params[:page])
-    @owner = Member.find_by_slug(params[:owner])
-    if @owner
-      @gardens = @owner.gardens.paginate(page: params[:page])
-    end
+    @owner = Member.find_by(slug: params[:owner])
+    @gardens = if @owner
+                 @owner.gardens.paginate(page: params[:page])
+               else
+                 Garden.paginate(page: params[:page])
+               end
 
     respond_to do |format|
       format.html # index.html.erb
@@ -20,8 +21,6 @@ class GardensController < ApplicationController
   # GET /gardens/1
   # GET /gardens/1.json
   def show
-    @garden = Garden.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
       format.json { render json: @garden }
@@ -41,7 +40,6 @@ class GardensController < ApplicationController
 
   # GET /gardens/1/edit
   def edit
-    @garden = Garden.find(params[:id])
   end
 
   # POST /gardens
@@ -65,8 +63,6 @@ class GardensController < ApplicationController
   # PUT /gardens/1
   # PUT /gardens/1.json
   def update
-    @garden = Garden.find(params[:id])
-
     respond_to do |format|
       if @garden.update(garden_params)
         format.html { redirect_to @garden, notice: 'Garden was successfully updated.' }
@@ -81,7 +77,6 @@ class GardensController < ApplicationController
   # DELETE /gardens/1
   # DELETE /gardens/1.json
   def destroy
-    @garden = Garden.find(params[:id])
     @garden.destroy
     expire_fragment("homepage_stats")
 

app/controllers/harvests_controller.rb

@@ -1,12 +1,12 @@
 class HarvestsController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /harvests
   # GET /harvests.json
   def index
-    @owner = Member.find_by_slug(params[:owner])
-    @crop = Crop.find_by_slug(params[:crop])
+    @owner = Member.find_by(slug: params[:owner])
+    @crop = Crop.find_by(slug: params[:crop])
     @harvests = if @owner
                   @owner.harvests.includes(:owner, :crop)
                 elsif @crop
@@ -32,7 +32,7 @@ class HarvestsController < ApplicationController
     @harvest = Harvest.new('harvested_at' => Date.today)
 
     # using find_by_id here because it returns nil, unlike find
-    @crop = Crop.find_by_id(params[:crop_id]) || Crop.new
+    @crop = Crop.find_or_initialize_by(id: params[:crop_id])
 
     respond_to do |format|
       format.html # new.html.erb
@@ -42,7 +42,6 @@ class HarvestsController < ApplicationController
 
   # GET /harvests/1/edit
   def edit
-    @harvest = Harvest.find(params[:id])
   end
 
   # POST /harvests
@@ -66,8 +65,6 @@ class HarvestsController < ApplicationController
   # PUT /harvests/1
   # PUT /harvests/1.json
   def update
-    @harvest = Harvest.find(params[:id])
-
     respond_to do |format|
       if @harvest.update(harvest_params)
         format.html { redirect_to @harvest, notice: 'Harvest was successfully updated.' }
@@ -82,7 +79,6 @@ class HarvestsController < ApplicationController
   # DELETE /harvests/1
   # DELETE /harvests/1.json
   def destroy
-    @harvest = Harvest.find(params[:id])
     @harvest.destroy
 
     respond_to do |format|

app/controllers/members_controller.rb

@@ -1,6 +1,5 @@
 class MembersController < ApplicationController
-  load_and_authorize_resource
-
+  load_and_authorize_resource except: [:finish_signup, :unsubscribe, :view_follows, :view_followers, :show]
   skip_authorize_resource only: [:nearby, :unsubscribe, :finish_signup]
 
   after_action :expire_cache_fragments, only: :create
@@ -17,7 +16,9 @@ class MembersController < ApplicationController
       format.html # index.html.haml
       format.json {
         render json: @members.to_json(only: [
-                                        :id, :login_name, :slug, :bio, :created_at, :location, :latitude, :longitude
+                                        :id, :login_name,
+                                        :slug, :bio, :created_at,
+                                        :location, :latitude, :longitude
                                       ])
       }
     end
@@ -38,7 +39,9 @@ class MembersController < ApplicationController
       format.html # show.html.haml
       format.json {
         render json: @member.to_json(only: [
-                                       :id, :login_name, :bio, :created_at, :slug, :location, :latitude, :longitude
+                                       :id, :login_name, :bio,
+                                       :created_at, :slug, :location,
+                                       :latitude, :longitude
                                      ])
       }
       format.rss { render(
@@ -64,32 +67,30 @@ class MembersController < ApplicationController
   }
 
   def unsubscribe
-    begin
-      verifier = ActiveSupport::MessageVerifier.new(ENV['RAILS_SECRET_TOKEN'])
-      decrypted_message = verifier.verify(params[:message])
+    verifier = ActiveSupport::MessageVerifier.new(ENV['RAILS_SECRET_TOKEN'])
+    decrypted_message = verifier.verify(params[:message])
 
-      @member = Member.find(decrypted_message[:member_id])
-      @type = decrypted_message[:type]
-      @member.update(@type => false)
+    @member = Member.find(decrypted_message[:member_id])
+    @type = decrypted_message[:type]
+    @member.update(@type => false)
 
-      flash.now[:notice] = "You have been unsubscribed from #{EMAIL_TYPE_STRING[@type]} emails."
+    flash.now[:notice] = "You have been unsubscribed from #{EMAIL_TYPE_STRING[@type]} emails."
 
-    rescue ActiveSupport::MessageVerifier::InvalidSignature
-      flash.now[:alert] = "We're sorry, there was an error updating your settings."
-    end
+  rescue ActiveSupport::MessageVerifier::InvalidSignature
+    flash.now[:alert] = "We're sorry, there was an error updating your settings."
   end
 
   def finish_signup
     @member = current_member
-    if request.patch? && params[:member]
-      if @member.update(member_params)
-        @member.skip_reconfirmation!
-        bypass_sign_in(@member)
-        redirect_to root_path, notice: 'Welcome.'
-      else
-        flash[:alert] = 'Failed to complete signup'
-        @show_errors = true
-      end
+    return unless request.patch? && params[:member]
+
+    if @member.update(member_params)
+      @member.skip_reconfirmation!
+      bypass_sign_in(@member)
+      redirect_to root_path, notice: 'Welcome.'
+    else
+      flash[:alert] = 'Failed to complete signup'
+      @show_errors = true
     end
   end
 

app/controllers/notifications_controller.rb

@@ -1,11 +1,11 @@
 class NotificationsController < ApplicationController
   include NotificationsHelper
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # GET /notifications
   def index
-    @notifications = Notification.where(recipient_id: current_member).page(params[:page])
+    @notifications = Notification.by_recipient(current_member).page(params[:page])
 
     respond_to do |format|
       format.html # index.html.erb
@@ -14,7 +14,6 @@ class NotificationsController < ApplicationController
 
   # GET /notifications/1
   def show
-    @notification = Notification.find(params[:id])
     @notification.read = true
     @notification.save
     @reply_link = reply_link(@notification)
@@ -28,7 +27,7 @@ class NotificationsController < ApplicationController
 
   def new
     @notification = Notification.new
-    @recipient = Member.find_by_id(params[:recipient_id])
+    @recipient = Member.find_by(id: params[:recipient_id])
     @subject   = params[:subject] || ""
 
     respond_to do |format|
@@ -54,7 +53,6 @@ class NotificationsController < ApplicationController
 
   # DELETE /notifications/1
   def destroy
-    @notification = Notification.find(params[:id])
     @notification.destroy
 
     respond_to do |format|
@@ -66,7 +64,7 @@ class NotificationsController < ApplicationController
   def create
     params[:notification][:sender_id] = current_member.id
     @notification = Notification.new(notification_params)
-    @recipient = Member.find_by_id(params[:notification][:recipient_id])
+    @recipient = Member.find_by(id: params[:notification][:recipient_id])
 
     respond_to do |format|
       if @notification.save

app/controllers/order_items_controller.rb

@@ -1,5 +1,5 @@
 class OrderItemsController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # POST /order_items

app/controllers/orders_controller.rb

@@ -1,10 +1,10 @@
 class OrdersController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # GET /orders
   def index
-    @orders = Order.where(member_id: current_member.id)
+    @orders = Order.by_member(current_member)
 
     respond_to do |format|
       format.html # index.html.erb
@@ -13,8 +13,6 @@ class OrdersController < ApplicationController
 
   # GET /orders/1
   def show
-    @order = Order.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
     end
@@ -31,8 +29,6 @@ class OrdersController < ApplicationController
 
   # checkout with PayPal
   def checkout
-    @order = Order.find(params[:id])
-
     respond_to do |format|
       if @order.update_attributes(referral_code: params[:referral_code])
         response = EXPRESS_GATEWAY.setup_purchase(
@@ -52,8 +48,6 @@ class OrdersController < ApplicationController
   end
 
   def complete
-    @order = Order.find(params[:id])
-
     if (params[:token] && params['PayerID'])
       purchase = EXPRESS_GATEWAY.purchase(
         @order.total,
@@ -80,7 +74,6 @@ class OrdersController < ApplicationController
   end
 
   def cancel
-    @order = Order.find(params[:id])
     respond_to do |format|
       format.html { redirect_to shop_url, notice: 'Order was cancelled.' }
     end
@@ -88,7 +81,6 @@ class OrdersController < ApplicationController
 
   # DELETE /orders/1
   def destroy
-    @order = Order.find(params[:id])
     @order.destroy
 
     respond_to do |format|

app/controllers/photos_controller.rb

@@ -41,7 +41,6 @@ class PhotosController < ApplicationController
 
   # GET /photos/1/edit
   def edit
-    @photo = Photo.find(params[:id])
   end
 
   # POST /photos
@@ -64,8 +63,6 @@ class PhotosController < ApplicationController
   # PUT /photos/1
   # PUT /photos/1.json
   def update
-    @photo = Photo.find(params[:id])
-
     respond_to do |format|
       if @photo.update(photo_params)
         format.html { redirect_to @photo, notice: 'Photo was successfully updated.' }
@@ -80,7 +77,6 @@ class PhotosController < ApplicationController
   # DELETE /photos/1
   # DELETE /photos/1.json
   def destroy
-    @photo = Photo.find(params[:id])
     @photo.destroy
     flash[:alert] = "Photo successfully deleted."
 
@@ -113,36 +109,17 @@ class PhotosController < ApplicationController
     @photo
   end
 
-  def which_collection?
-    case params[:type]
-    when "garden" then @photo.gardens
-    when "harvest" then @photo.harvests
-    when "planting" then @photo.plantings
-    else raise "Invalid type"
-    end
-  end
-
   def add_photo_to_collection
-    collection = which_collection?
+    raise "Missing or invalid type provided" unless Growstuff::Constants::PhotoModels.types.include?(params[:type])
+    raise "No item id provided" unless item_id?
+    collection = Growstuff::Constants::PhotoModels.get_relation(@photo, params[:type])
 
-    unless collection && item_id?
-      flash[:alert] = "Missing or invalid type or id parameter"
-      return
-    end
+    item_class = Growstuff::Constants::PhotoModels.get_item(params[:type])
+    item = item_class.find_by!(id: params[:id], owner_id: current_member.id)
+    raise "Could not find this item owned by you" unless item
 
-    item = find_item_for_photo!
     collection << item unless collection.include?(item)
-  rescue
-    flash[:alert] = "Could not find this item owned by you"
-  end
-
-  def find_item_for_photo!
-    item_class = case params[:type]
-                 when "garden" then Garden
-                 when "harvest" then Harvest
-                 when "planting" then Planting
-                 else raise "Invalid type"
-                 end
-    item_class.find_by!(id: params[:id], owner_id: current_member.id)
+  rescue => e
+    flash[:alert] = e.message
   end
 end

app/controllers/plant_parts_controller.rb

@@ -15,8 +15,6 @@ class PlantPartsController < ApplicationController
   # GET /plant_parts/1
   # GET /plant_parts/1.json
   def show
-    @plant_part = PlantPart.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
       format.json { render json: @plant_part }
@@ -36,7 +34,6 @@ class PlantPartsController < ApplicationController
 
   # GET /plant_parts/1/edit
   def edit
-    @plant_part = PlantPart.find(params[:id])
   end
 
   # POST /plant_parts
@@ -58,8 +55,6 @@ class PlantPartsController < ApplicationController
   # PUT /plant_parts/1
   # PUT /plant_parts/1.json
   def update
-    @plant_part = PlantPart.find(params[:id])
-
     respond_to do |format|
       if @plant_part.update(plant_part_params)
         format.html { redirect_to @plant_part, notice: 'Plant part was successfully updated.' }
@@ -74,7 +69,6 @@ class PlantPartsController < ApplicationController
   # DELETE /plant_parts/1
   # DELETE /plant_parts/1.json
   def destroy
-    @plant_part = PlantPart.find(params[:id])
     @plant_part.destroy
 
     respond_to do |format|

app/controllers/plantings_controller.rb

@@ -1,12 +1,12 @@
 class PlantingsController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /plantings
   # GET /plantings.json
   def index
-    @owner = Member.find_by_slug(params[:owner])
-    @crop = Crop.find_by_slug(params[:crop])
+    @owner = Member.find_by(slug: params[:owner])
+    @crop = Crop.find_by(slug: params[:crop])
     @plantings = if @owner
                    @owner.plantings.includes(:owner, :crop, :garden).paginate(page: params[:page])
                  elsif @crop
@@ -44,8 +44,8 @@ class PlantingsController < ApplicationController
     @planting = Planting.new('planted_at' => Date.today)
 
     # using find_by_id here because it returns nil, unlike find
-    @crop     = Crop.find_by_id(params[:crop_id])     || Crop.new
-    @garden   = Garden.find_by_id(params[:garden_id]) || Garden.new
+    @crop     = Crop.find_by(id: params[:crop_id])     || Crop.new
+    @garden   = Garden.find_by(id: params[:garden_id]) || Garden.new
 
     respond_to do |format|
       format.html # new.html.erb
@@ -55,8 +55,6 @@ class PlantingsController < ApplicationController
 
   # GET /plantings/1/edit
   def edit
-    @planting = Planting.find(params[:id])
-
     # the following are needed to display the form but aren't used
     @crop     = Crop.new
     @garden   = Garden.new
@@ -86,7 +84,6 @@ class PlantingsController < ApplicationController
   # PUT /plantings/1
   # PUT /plantings/1.json
   def update
-    @planting = Planting.find(params[:id])
     params[:planted_at] = parse_date(params[:planted_at])
 
     respond_to do |format|
@@ -105,7 +102,6 @@ class PlantingsController < ApplicationController
   # DELETE /plantings/1
   # DELETE /plantings/1.json
   def destroy
-    @planting = Planting.find(params[:id])
     @garden = @planting.garden
     @planting.destroy
     expire_fragment("homepage_stats")

app/controllers/posts_controller.rb

@@ -1,12 +1,12 @@
 class PostsController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /posts
   # GET /posts.json
 
   def index
-    @author = Member.find_by_slug(params[:author])
+    @author = Member.find_by(slug: params[:author])
     @posts = if @author
                @author.posts.includes(:author, { comments: :author }).paginate(page: params[:page])
              else
@@ -39,7 +39,7 @@ class PostsController < ApplicationController
   # GET /posts/new.json
   def new
     @post = Post.new
-    @forum = Forum.find_by_id(params[:forum_id])
+    @forum = Forum.find_by(id: params[:forum_id])
 
     respond_to do |format|
       format.html # new.html.haml
@@ -49,7 +49,6 @@ class PostsController < ApplicationController
 
   # GET /posts/1/edit
   def edit
-    @post = Post.find(params[:id])
   end
 
   # POST /posts
@@ -72,8 +71,6 @@ class PostsController < ApplicationController
   # PUT /posts/1
   # PUT /posts/1.json
   def update
-    @post = Post.find(params[:id])
-
     respond_to do |format|
       if @post.update(post_params)
         format.html { redirect_to @post, notice: 'Post was successfully updated.' }
@@ -88,7 +85,6 @@ class PostsController < ApplicationController
   # DELETE /posts/1
   # DELETE /posts/1.json
   def destroy
-    @post = Post.find(params[:id])
     @post.destroy
 
     respond_to do |format|

app/controllers/products_controller.rb

@@ -1,5 +1,5 @@
 class ProductsController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # GET /products
@@ -13,8 +13,6 @@ class ProductsController < ApplicationController
 
   # GET /products/1
   def show
-    @product = Product.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
     end
@@ -31,7 +29,6 @@ class ProductsController < ApplicationController
 
   # GET /products/1/edit
   def edit
-    @product = Product.find(params[:id])
   end
 
   # POST /products
@@ -49,8 +46,6 @@ class ProductsController < ApplicationController
 
   # PUT /products/1
   def update
-    @product = Product.find(params[:id])
-
     respond_to do |format|
       if @product.update(product_params)
         format.html { redirect_to @product, notice: 'Product was successfully updated.' }
@@ -62,7 +57,6 @@ class ProductsController < ApplicationController
 
   # DELETE /products/1
   def destroy
-    @product = Product.find(params[:id])
     @product.destroy
 
     respond_to do |format|

app/controllers/roles_controller.rb

@@ -1,5 +1,5 @@
 class RolesController < ApplicationController
-  before_filter :authenticate_member!
+  before_action :authenticate_member!
   load_and_authorize_resource
 
   # GET /roles
@@ -13,8 +13,6 @@ class RolesController < ApplicationController
 
   # GET /roles/1
   def show
-    @role = Role.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
     end
@@ -31,7 +29,6 @@ class RolesController < ApplicationController
 
   # GET /roles/1/edit
   def edit
-    @role = Role.find(params[:id])
   end
 
   # POST /roles
@@ -49,8 +46,6 @@ class RolesController < ApplicationController
 
   # PUT /roles/1
   def update
-    @role = Role.find(params[:id])
-
     respond_to do |format|
       if @role.update(role_params)
         format.html { redirect_to @role, notice: 'Role was successfully updated.' }
@@ -62,7 +57,6 @@ class RolesController < ApplicationController
 
   # DELETE /roles/1
   def destroy
-    @role = Role.find(params[:id])
     @role.destroy
 
     respond_to do |format|

app/controllers/scientific_names_controller.rb

@@ -1,5 +1,5 @@
 class ScientificNamesController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /scientific_names
@@ -16,8 +16,6 @@ class ScientificNamesController < ApplicationController
   # GET /scientific_names/1
   # GET /scientific_names/1.json
   def show
-    @scientific_name = ScientificName.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.haml
       format.json { render json: @scientific_name }
@@ -28,7 +26,7 @@ class ScientificNamesController < ApplicationController
   # GET /scientific_names/new.json
   def new
     @scientific_name = ScientificName.new
-    @crop = Crop.find_by_id(params[:crop_id]) || Crop.new
+    @crop = Crop.find_or_initialize_by(id: params[:crop_id])
 
     respond_to do |format|
       format.html # new.html.haml
@@ -38,7 +36,6 @@ class ScientificNamesController < ApplicationController
 
   # GET /scientific_names/1/edit
   def edit
-    @scientific_name = ScientificName.find(params[:id])
   end
 
   # POST /scientific_names
@@ -61,8 +58,6 @@ class ScientificNamesController < ApplicationController
   # PUT /scientific_names/1
   # PUT /scientific_names/1.json
   def update
-    @scientific_name = ScientificName.find(params[:id])
-
     respond_to do |format|
       if @scientific_name.update(scientific_name_params)
         format.html { redirect_to @scientific_name.crop, notice: 'Scientific name was successfully updated.' }
@@ -77,7 +72,6 @@ class ScientificNamesController < ApplicationController
   # DELETE /scientific_names/1
   # DELETE /scientific_names/1.json
   def destroy
-    @scientific_name = ScientificName.find(params[:id])
     @crop = @scientific_name.crop
     @scientific_name.destroy
 

app/controllers/seeds_controller.rb

@@ -1,12 +1,12 @@
 class SeedsController < ApplicationController
-  before_filter :authenticate_member!, except: [:index, :show]
+  before_action :authenticate_member!, except: [:index, :show]
   load_and_authorize_resource
 
   # GET /seeds
   # GET /seeds.json
   def index
-    @owner = Member.find_by_slug(params[:owner])
-    @crop = Crop.find_by_slug(params[:crop])
+    @owner = Member.find_by(slug: params[:owner])
+    @crop = Crop.find_by(slug: params[:crop])
     @seeds = if @owner
                @owner.seeds.includes(:owner, :crop).paginate(page: params[:page])
              elsif @crop
@@ -35,8 +35,6 @@ class SeedsController < ApplicationController
   # GET /seeds/1
   # GET /seeds/1.json
   def show
-    @seed = Seed.find(params[:id])
-
     respond_to do |format|
       format.html # show.html.erb
       format.json { render json: @seed }
@@ -49,7 +47,7 @@ class SeedsController < ApplicationController
     @seed = Seed.new
 
     # using find_by_id here because it returns nil, unlike find
-    @crop = Crop.find_by_id(params[:crop_id]) || Crop.new
+    @crop = Crop.find_or_initialize_by(id: params[:crop_id])
 
     respond_to do |format|
       format.html # new.html.erb
@@ -59,7 +57,6 @@ class SeedsController < ApplicationController
 
   # GET /seeds/1/edit
   def edit
-    @seed = Seed.find(params[:id])
   end
 
   # POST /seeds
@@ -82,8 +79,6 @@ class SeedsController < ApplicationController
   # PUT /seeds/1
   # PUT /seeds/1.json
   def update
-    @seed = Seed.find(params[:id])
-
     respond_to do |format|
       if @seed.update(seed_params)
         format.html { redirect_to @seed, notice: 'Seed was successfully updated.' }
@@ -98,7 +93,6 @@ class SeedsController < ApplicationController
   # DELETE /seeds/1
   # DELETE /seeds/1.json
   def destroy
-    @seed = Seed.find(params[:id])
     @seed.destroy
 
     respond_to do |format|

app/helpers/application_helper.rb

@@ -1,17 +1,16 @@
 module ApplicationHelper
   def price_in_dollars(price)
-    return sprintf('%.2f', price / 100.0)
+    sprintf('%.2f', price / 100.0)
   end
 
   # 999 cents becomes 9.99 AUD -- for products/orders/etc
   def price_with_currency(price)
-    return sprintf('%.2f %s', price / 100.0,
-      Growstuff::Application.config.currency)
+    sprintf('%.2f %s', price / 100.0, Growstuff::Application.config.currency)
   end
 
   def parse_date(str)
     str ||= '' # Date.parse barfs on nil
-    return str == '' ? nil : Date.parse(str)
+    str == '' ? nil : Date.parse(str)
   end
 
   def forex_link(price)

app/helpers/harvests_helper.rb

@@ -3,44 +3,32 @@ module HarvestsHelper
     human_quantity = display_human_quantity(harvest)
     weight = display_weight(harvest)
 
-    if human_quantity && weight
-      return "#{human_quantity}, weighing #{weight}"
-    elsif human_quantity
-      return human_quantity
-    elsif weight
-      return weight
-    else
-      return 'not specified'
-    end
+    return "#{human_quantity}, weighing #{weight}" if human_quantity && weight
+    return human_quantity if human_quantity
+    return weight if weight
+
+    'not specified'
   end
 
   def display_human_quantity(harvest)
-    if !harvest.quantity.blank? && harvest.quantity > 0
-      if harvest.unit == 'individual' # just the number
-        number_to_human(harvest.quantity, strip_insignificant_zeros: true)
-      elsif !harvest.unit.blank? # pluralize anything else
-        return pluralize(number_to_human(harvest.quantity, strip_insignificant_zeros: true), harvest.unit)
-      else
-        return "#{number_to_human(harvest.quantity, strip_insignificant_zeros: true)} #{harvest.unit}"
-      end
+    return unless harvest.quantity.present? && harvest.quantity > 0
+
+    if harvest.unit == 'individual' # just the number
+      number_to_human(harvest.quantity, strip_insignificant_zeros: true)
+    elsif !harvest.unit.blank? # pluralize anything else
+      pluralize(number_to_human(harvest.quantity, strip_insignificant_zeros: true), harvest.unit)
     else
-      return nil
+      "#{number_to_human(harvest.quantity, strip_insignificant_zeros: true)} #{harvest.unit}"
     end
   end
 
   def display_weight(harvest)
-    if !harvest.weight_quantity.blank? && harvest.weight_quantity > 0
-      return "#{number_to_human(harvest.weight_quantity, strip_insignificant_zeros: true)} #{harvest.weight_unit}"
-    else
-      return nil
-    end
+    return if harvest.weight_quantity.blank? || harvest.weight_quantity <= 0
+    "#{number_to_human(harvest.weight_quantity, strip_insignificant_zeros: true)} #{harvest.weight_unit}"
   end
 
   def display_harvest_description(harvest)
-    if harvest.description.empty?
-      "No description provided."
-    else
-      harvest.description
-    end
+    return "No description provided." if harvest.description.empty?
+    harvest.description
   end
 end

app/helpers/plantings_helper.rb

@@ -1,13 +1,13 @@
 module PlantingsHelper
   def display_days_before_maturity(planting)
     if planting.finished?
-      0
+      "0"
     elsif !planting.finished_at.nil?
-      ((p = planting.finished_at - DateTime.now).to_i) <= 0 ? 0 : p.to_i
+      ((p = planting.finished_at - Date.current).to_i) <= 0 ? "0" : p.to_i.to_s
     elsif planting.planted_at.nil? || planting.days_before_maturity.nil?
       "unknown"
     else
-      ((p = (planting.planted_at + planting.days_before_maturity) - DateTime.now).to_i <= 0) ? 0 : p.to_i
+      ((p = (planting.planted_at + planting.days_before_maturity) - Date.current).to_i <= 0) ? "0" : p.to_i.to_s
     end
   end
 
@@ -31,13 +31,13 @@ module PlantingsHelper
 
   def display_planting(planting)
     if planting.quantity.to_i > 0 && planting.planted_from.present?
-      return "#{planting.owner} planted #{pluralize(planting.quantity, planting.planted_from)}."
+      "#{planting.owner} planted #{pluralize(planting.quantity, planting.planted_from)}."
     elsif planting.quantity.to_i > 0
-      return "#{planting.owner} planted #{pluralize(planting.quantity, 'unit')}."
+      "#{planting.owner} planted #{pluralize(planting.quantity, 'unit')}."
     elsif planting.planted_from.present?
-      return "#{planting.owner} planted #{planting.planted_from.pluralize}."
+      "#{planting.owner} planted #{planting.planted_from.pluralize}."
     else
-      return "#{planting.owner}."
+      "#{planting.owner}."
     end
   end
 end

app/mailers/notifier.rb

@@ -8,7 +8,7 @@ class Notifier < ActionMailer::Base
         "not set - have you created config/application.yml?"
     end
 
-    return ActiveSupport::MessageVerifier.new(ENV['RAILS_SECRET_TOKEN'])
+    ActiveSupport::MessageVerifier.new(ENV['RAILS_SECRET_TOKEN'])
   end
 
   def notify(notification)
@@ -16,7 +16,8 @@ class Notifier < ActionMailer::Base
     @reply_link = reply_link(@notification)
 
     # Encrypting
-    @signed_message = verifier.generate ({ member_id: @notification.recipient.id, type: :send_notification_email })
+    message = { member_id: @notification.recipient.id, type: :send_notification_email }
+    @signed_message = verifier.generate(message)
 
     mail(to: @notification.recipient.email,
          subject: @notification.subject)
@@ -29,12 +30,10 @@ class Notifier < ActionMailer::Base
     @harvests = @member.harvests.first(5)
 
     # Encrypting
-    @signed_message = verifier.generate ({ member_id: @member.id, type: :send_planting_reminder })
+    message = { member_id: @member.id, type: :send_planting_reminder }
+    @signed_message = verifier.generate(message)
 
-    if @member.send_planting_reminder
-      mail(to: @member.email,
-           subject: "What have you planted lately?")
-    end
+    mail(to: @member.email, subject: "What have you planted lately?") if @member.send_planting_reminder
   end
 
   def new_crop_request(member, request)

app/models/ability.rb

@@ -1,7 +1,7 @@
 class Ability
   include CanCan::Ability
 
-  def initialize(member)
+  def initialize(member) # rubocop:disable Metrics/AbcSize
     # See the wiki for details: https://github.com/ryanb/cancan/wiki/Defining-Abilities
 
     # everyone can do these things, even non-logged in
@@ -36,110 +36,107 @@ class Ability
       an.crop.approved?
     end
 
-    if member
-      # members can see even rejected or pending crops if they requested it
-      can :read, Crop, requester_id: member.id
+    return unless member
 
-      # managing your own user settings
-      can :update, Member, id: member.id
+    # members can see even rejected or pending crops if they requested it
+    can :read, Crop, requester_id: member.id
 
-      # can read/delete notifications that were sent to them
-      can :read, Notification, recipient_id: member.id
-      can :destroy, Notification, recipient_id: member.id
-      can :reply, Notification, recipient_id: member.id
-      # can send a private message to anyone but themselves
-      # note: sadly, we can't test for this from the view, but it works
-      # for the model/controller
-      can :create, Notification do |n|
-        n.recipient_id != member.id
-      end
-      # note we don't support update for notifications
+    # managing your own user settings
+    can :update, Member, id: member.id
 
-      # only crop wranglers can create/edit/destroy crops
-      if member.has_role? :crop_wrangler
-        can :wrangle, Crop
-        can :manage, Crop
-        can :manage, ScientificName
-        can :manage, AlternateName
-      end
+    # can read/delete notifications that were sent to them
+    can :read, Notification, recipient_id: member.id
+    can :destroy, Notification, recipient_id: member.id
+    can :reply, Notification, recipient_id: member.id
+    # can send a private message to anyone but themselves
+    # note: sadly, we can't test for this from the view, but it works
+    # for the model/controller
+    can :create, Notification do |n|
+      n.recipient_id != member.id
+    end
+    # note we don't support update for notifications
 
-      # any member can create a crop provisionally
-      can :create, Crop
+    # only crop wranglers can create/edit/destroy crops
+    if member.has_role? :crop_wrangler
+      can :wrangle, Crop
+      can :manage, Crop
+      can :manage, ScientificName
+      can :manage, AlternateName
+    end
 
-      # can create & destroy their own authentications against other sites.
-      can :create, Authentication
-      can :destroy, Authentication, member_id: member.id
+    # any member can create a crop provisionally
+    can :create, Crop
 
-      # anyone can create a post, or comment on a post,
-      # but only the author can edit/destroy it.
-      can :create,  Post
-      can :update,  Post, author_id: member.id
-      can :destroy, Post, author_id: member.id
-      can :create,  Comment
-      can :update,  Comment, author_id: member.id
-      can :destroy, Comment, author_id: member.id
+    # can create & destroy their own authentications against other sites.
+    can :create, Authentication
+    can :destroy, Authentication, member_id: member.id
 
-      # same deal for gardens and plantings
-      can :create,  Garden
-      can :update,  Garden, owner_id: member.id
-      can :destroy, Garden, owner_id: member.id
+    # anyone can create a post, or comment on a post,
+    # but only the author can edit/destroy it.
+    can :create,  Post
+    can :update,  Post, author_id: member.id
+    can :destroy, Post, author_id: member.id
+    can :create,  Comment
+    can :update,  Comment, author_id: member.id
+    can :destroy, Comment, author_id: member.id
 
-      can :create,  Planting
-      can :update,  Planting, garden: { owner_id: member.id }
-      can :destroy, Planting, garden: { owner_id: member.id }
+    # same deal for gardens and plantings
+    can :create,  Garden
+    can :update,  Garden, owner_id: member.id
+    can :destroy, Garden, owner_id: member.id
 
-      can :create,  Harvest
-      can :update,  Harvest, owner_id: member.id
-      can :destroy, Harvest, owner_id: member.id
+    can :create,  Planting
+    can :update,  Planting, garden: { owner_id: member.id }
+    can :destroy, Planting, garden: { owner_id: member.id }
 
-      can :create, Photo
-      can :update, Photo, owner_id: member.id
-      can :destroy, Photo, owner_id: member.id
+    can :create,  Harvest
+    can :update,  Harvest, owner_id: member.id
+    can :destroy, Harvest, owner_id: member.id
 
-      can :create, Seed
-      can :update, Seed, owner_id: member.id
-      can :destroy, Seed, owner_id: member.id
+    can :create, Photo
+    can :update, Photo, owner_id: member.id
+    can :destroy, Photo, owner_id: member.id
 
-      # orders/shop/etc
-      can :create,   Order
-      can :read,     Order, member_id: member.id
-      can :complete, Order, member_id: member.id, completed_at: nil
-      can :checkout, Order, member_id: member.id, completed_at: nil
-      can :cancel,   Order, member_id: member.id, completed_at: nil
-      can :destroy,  Order, member_id: member.id, completed_at: nil
+    can :create, Seed
+    can :update, Seed, owner_id: member.id
+    can :destroy, Seed, owner_id: member.id
 
-      can :create, OrderItem
-      # for now, let's not let people mess with individual order items
-      cannot :read,    OrderItem, order: { member_id: member.id }
-      cannot :update,  OrderItem, order: { member_id: member.id, completed_at: nil }
-      cannot :destroy, OrderItem, order: { member_id: member.id, completed_at: nil }
+    # orders/shop/etc
+    can :create,   Order
+    can :read,     Order, member_id: member.id
+    can :complete, Order, member_id: member.id, completed_at: nil
+    can :checkout, Order, member_id: member.id, completed_at: nil
+    can :cancel,   Order, member_id: member.id, completed_at: nil
+    can :destroy,  Order, member_id: member.id, completed_at: nil
 
-      # following/unfollowing permissions
-      can :create, Follow
-      cannot :create, Follow, followed_id: member.id # can't follow yourself
+    can :create, OrderItem
+    # for now, let's not let people mess with individual order items
+    cannot :read,    OrderItem, order: { member_id: member.id }
+    cannot :update,  OrderItem, order: { member_id: member.id, completed_at: nil }
+    cannot :destroy, OrderItem, order: { member_id: member.id, completed_at: nil }
 
-      can :destroy, Follow
-      cannot :destroy, Follow, followed_id: member.id # can't unfollow yourself
+    # following/unfollowing permissions
+    can :create, Follow
+    cannot :create, Follow, followed_id: member.id # can't follow yourself
 
-      if member.has_role? :admin
+    can :destroy, Follow
+    cannot :destroy, Follow, followed_id: member.id # can't unfollow yourself
 
-        can :read, :all
-        can :manage, :all
+    return unless member.has_role? :admin
 
-        # can't change order history, because it's *history*
-        cannot :create, Order
-        cannot :complete, Order
-        cannot :destroy, Order
-        cannot :manage, OrderItem
+    can :read, :all
+    can :manage, :all
 
-        # can't delete plant parts if they have harvests associated with them
-        cannot :destroy, PlantPart
-        can :destroy, PlantPart do |pp|
-          pp.harvests.empty?
-        end
-
-      end
+    # can't change order history, because it's *history*
+    cannot :create, Order
+    cannot :complete, Order
+    cannot :destroy, Order
+    cannot :manage, OrderItem
 
+    # can't delete plant parts if they have harvests associated with them
+    cannot :destroy, PlantPart
+    can :destroy, PlantPart do |pp|
+      pp.harvests.empty?
     end
   end
 end

app/models/account.rb

@@ -16,9 +16,9 @@ class Account < ActiveRecord::Base
 
   def paid_until_string
     if account_type.is_permanent_paid
-      return "forever"
+      "forever"
     elsif account_type.is_paid
-      return paid_until.to_s
+      paid_until.to_s
     end
   end
 end

app/models/concerns/photo_capable.rb

@@ -0,0 +1,17 @@
+require_relative '../../constants/photo_models.rb'
+module PhotoCapable
+  extend ActiveSupport::Concern
+
+  included do
+    has_and_belongs_to_many :photos # rubocop:disable Rails/HasAndBelongsToMany
+
+    before_destroy :remove_from_list
+  end
+
+  def remove_from_list
+    photolist = photos.to_a # save a temp copy of the photo list
+    photos.clear # clear relationship b/w object and photo
+
+    photolist.each(&:destroy_if_unused)
+  end
+end

app/models/crop.rb

@@ -1,4 +1,4 @@
-class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
+class Crop < ActiveRecord::Base
   extend FriendlyId
   friendly_id :name, use: [:slugged, :finders]
 
@@ -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\/[[:alnum:]%_]+\z/,
+      with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:]%_\.()-]+\z/,
       message: 'is not a valid English Wikipedia URL'
     },
     if: :approved?
@@ -115,9 +115,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
   # update the Elasticsearch index (only if we're using it in this
   # environment)
   def update_index(name_obj)
-    if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
-      __elasticsearch__.index_document
-    end
+    __elasticsearch__.index_document if ENV["GROWSTUFF_ELASTICSEARCH"] == "true"
   end
 
   # End Elasticsearch section
@@ -127,9 +125,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
   end
 
   def default_scientific_name
-    if scientific_names.size > 0
-      scientific_names.first.name
-    end
+    scientific_names.first.name if scientific_names.size > 0
   end
 
   # crop.default_photo
@@ -168,11 +164,9 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
   def popular_plant_parts
     popular_plant_parts = Hash.new(0)
     harvests.each do |h|
-      if h.plant_part
-        popular_plant_parts[h.plant_part] += 1
-      end
+      popular_plant_parts[h.plant_part] += 1 if h.plant_part
     end
-    return popular_plant_parts
+    popular_plant_parts
   end
 
   def interesting?
@@ -180,7 +174,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
     min_photos    = 3 # needs this many photos to be interesting
     return false unless photos.size >= min_photos
     return false unless plantings_count >= min_plantings
-    return true
+    true
   end
 
   def pending?
@@ -213,7 +207,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
       next unless c.interesting?
       interesting_crops.push(c)
     end
-    return interesting_crops
+    interesting_crops
   end
 
   # Crop.create_from_csv(row)
@@ -227,7 +221,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
   def Crop.create_from_csv(row)
     name, en_wikipedia_url, parent, scientific_names, alternate_names = row
 
-    cropbot = Member.find_by_login_name('cropbot')
+    cropbot = Member.find_by(login_name: 'cropbot')
     raise "cropbot account not found: run rake db:seed" unless cropbot
 
     crop = Crop.find_or_create_by(name: name)
@@ -237,7 +231,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
     )
 
     if parent
-      parent = Crop.find_by_name(parent)
+      parent = Crop.find_by(name: parent)
       if parent
         crop.update_attributes(parent_id: parent.id)
       else
@@ -259,34 +253,28 @@ 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')
+    cropbot = Member.find_by(login_name: 'cropbot')
 
-    if names_to_add.size > 0
-      raise "cropbot account not found: run rake db:seed" unless cropbot
+    return unless names_to_add.size > 0
+    raise "cropbot account not found: run rake db:seed" unless cropbot
 
-      add_names_to_list(names_to_add, 'scientific')
-    end
+    add_names_to_list(names_to_add, 'scientific')
   end
 
   def add_alternate_names_from_csv(alternate_names)
-    cropbot = Member.find_by_login_name('cropbot')
+    # i.e. we actually passed something in, which isn't a given
+    return if alternate_names.blank?
 
-    names_to_add = []
-
-    if !alternate_names.blank? # i.e. we actually passed something in, which isn't a given
-      raise "cropbot account not found: run rake db:seed" unless cropbot
-
-      names_to_add = alternate_names.split(%r{,\s*})
-      add_names_to_list(names_to_add, 'alternate')
-    end
+    cropbot = Member.find_by!(login_name: 'cropbot')
+    names_to_add = alternate_names.split(%r{,\s*})
+    add_names_to_list(names_to_add, 'alternate')
+  rescue
+    raise "cropbot account not found: run rake db:seed" unless cropbot
   end
 
   def rejection_explanation
-    if reason_for_rejection == "other"
-      return rejection_notes
-    else
-      return reason_for_rejection
-    end
+    return rejection_notes if reason_for_rejection == "other"
+    reason_for_rejection
   end
 
   # Crop.search(string)
@@ -311,7 +299,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
                                             size: 50
                                           }
       )
-      return response.records.to_a
+      response.records.to_a
     else
       # if we don't have elasticsearch, just do a basic SQL query.
       # also, make sure it's an actual array not an activerecord
@@ -322,16 +310,20 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
 
       # we want to make sure that exact matches come first, even if not
       # using elasticsearch (eg. in development)
-      exact_match = Crop.approved.find_by_name(query)
+      exact_match = Crop.approved.find_by(name: query)
       if exact_match
         matches.delete(exact_match)
         matches.unshift(exact_match)
       end
 
-      return matches
+      matches
     end
   end
 
+  def Crop.case_insensitive_name(name)
+    where(["lower(name) = :value", { value: name.downcase }])
+  end
+
   private
 
   def add_names_to_list(names_to_add, list_name)
@@ -345,7 +337,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
   end
 
   def create_crop_in_list(list_name, name)
-    cropbot = Member.find_by_login_name('cropbot')
+    cropbot = Member.find_by(login_name: 'cropbot')
     create_hash = {
       creator_id: "#{cropbot.id}",
       name: name
@@ -360,9 +352,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
   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
+      data[p.send("#{col_name}")] += 1 if !p.send("#{col_name}").blank?
     end
     data
   end
@@ -371,22 +361,18 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength
 
   def approval_status_cannot_be_changed_again
     previous = previous_changes.include?(:approval_status) ? previous_changes.approval_status : {}
-    if previous.include?(:rejected) || previous.include?(:approved)
-      errors.add(:approval_status, "has already been set to #{approval_status}")
-    end
+    return unless previous.include?(:rejected) || previous.include?(:approved)
+    errors.add(:approval_status, "has already been set to #{approval_status}")
   end
 
   def must_be_rejected_if_rejected_reasons_present
-    unless rejected?
-      if reason_for_rejection.present? || rejection_notes.present?
-        errors.add(:approval_status, "must be rejected if a reason for rejection is present")
-      end
-    end
+    return if rejected?
+    return unless reason_for_rejection.present? || rejection_notes.present?
+    errors.add(:approval_status, "must be rejected if a reason for rejection is present")
   end
 
   def must_have_meaningful_reason_for_rejection
-    if reason_for_rejection == "other" && rejection_notes.blank?
-      errors.add(:rejection_notes, "must be added if the reason for rejection is \"other\"")
-    end
+    return unless reason_for_rejection == "other" && rejection_notes.blank?
+    errors.add(:rejection_notes, "must be added if the reason for rejection is \"other\"")
   end
 end

app/models/forum.rb

@@ -6,6 +6,6 @@ class Forum < ActiveRecord::Base
   belongs_to :owner, class_name: "Member"
 
   def to_s
-    return name
+    name
   end
 end

app/models/garden.rb

@@ -1,23 +1,13 @@
 class Garden < ActiveRecord::Base
-  include Geocodable
   extend FriendlyId
+  include Geocodable
+  include PhotoCapable
   friendly_id :garden_slug, use: [:slugged, :finders]
 
   belongs_to :owner, class_name: 'Member', foreign_key: 'owner_id'
   has_many :plantings, -> { order(created_at: :desc) }, dependent: :destroy
   has_many :crops, through: :plantings
 
-  has_and_belongs_to_many :photos
-
-  before_destroy do |garden|
-    photolist = garden.photos.to_a # save a temp copy of the photo list
-    garden.photos.clear # clear relationship b/w garden and photo
-
-    photolist.each do |photo|
-      photo.destroy_if_unused
-    end
-  end
-
   # set up geocoding
   geocoded_by :location
   after_validation :geocode
@@ -57,12 +47,8 @@ class Garden < ActiveRecord::Base
   after_validation :cleanup_area
 
   def cleanup_area
-    if area == 0
-      self.area = nil
-    end
-    if area.blank?
-      self.area_unit = nil
-    end
+    self.area = nil if area == 0
+    self.area_unit = nil if area.blank?
   end
 
   def garden_slug
@@ -82,7 +68,7 @@ class Garden < ActiveRecord::Base
       end
     end
 
-    return unique_plantings[0..3]
+    unique_plantings[0..3]
   end
 
   def to_s
@@ -92,15 +78,15 @@ class Garden < ActiveRecord::Base
   # When you mark a garden as inactive, all the plantings in it should be
   # marked as finished.  This automates that.
   def mark_inactive_garden_plantings_as_finished
-    if (active == false)
-      plantings.current.each do |p|
-        p.finished = true
-        p.save
-      end
+    return unless active == false
+
+    plantings.current.each do |p|
+      p.finished = true
+      p.save
     end
   end
 
   def default_photo
-    return photos.first
+    photos.first
   end
 end

app/models/harvest.rb

@@ -1,23 +1,13 @@
 class Harvest < ActiveRecord::Base
-  include ActionView::Helpers::NumberHelper
   extend FriendlyId
+  include ActionView::Helpers::NumberHelper
+  include PhotoCapable
   friendly_id :harvest_slug, use: [:slugged, :finders]
 
   belongs_to :crop
   belongs_to :owner, class_name: 'Member'
   belongs_to :plant_part
 
-  has_and_belongs_to_many :photos
-
-  before_destroy do |harvest|
-    photolist = harvest.photos.to_a # save a temp copy of the photo list
-    harvest.photos.clear # clear relationship b/w harvest and photo
-
-    photolist.each do |photo|
-      photo.destroy_if_unused
-    end
-  end
-
   default_scope { order('created_at DESC') }
 
   validates :crop, approved: true
@@ -70,28 +60,16 @@ class Harvest < ActiveRecord::Base
   # we're storing the harvest weight in kilograms in the db too
   # to make data manipulation easier
   def set_si_weight
-    if self.weight_unit != nil
-      weight_string = "#{self.weight_quantity} #{self.weight_unit}"
-      self.si_weight = Unit.new(weight_string).convert_to("kg").to_s("%0.3f").delete(" kg").to_f
-    end
+    return if self.weight_unit.nil?
+    weight_string = "#{self.weight_quantity} #{self.weight_unit}"
+    self.si_weight = Unit.new(weight_string).convert_to("kg").to_s("%0.3f").delete(" kg").to_f
   end
 
   def cleanup_quantities
-    if quantity == 0
-      self.quantity = nil
-    end
-
-    if quantity.blank?
-      self.unit = nil
-    end
-
-    if weight_quantity == 0
-      self.weight_quantity = nil
-    end
-
-    if weight_quantity.blank?
-      self.weight_unit = nil
-    end
+    self.quantity = nil if quantity == 0
+    self.unit = nil if quantity.blank?
+    self.weight_quantity = nil if weight_quantity == 0
+    self.weight_unit = nil if weight_quantity.blank?
   end
 
   def harvest_slug
@@ -127,10 +105,10 @@ class Harvest < ActiveRecord::Base
         " #{self.weight_unit}"
     end
 
-    return string
+    string
   end
 
   def default_photo
-    return photos.first || crop.default_photo
+    photos.first || crop.default_photo
   end
 end

app/models/member.rb

@@ -91,15 +91,13 @@ class Member < ActiveRecord::Base
   # allow login via either login_name or email address
   def self.find_first_by_auth_conditions(warden_conditions)
     conditions = warden_conditions.dup
-    if login = conditions.delete(:login)
-      where(conditions).where(["lower(login_name) = :value OR lower(email) = :value", { value: login.downcase }]).first
-    else
-      where(conditions).first
-    end
+    login = conditions.delete(:login)
+    return  where(conditions).login_name_or_email(login).first if login
+    find_by(conditions)
   end
 
   def to_s
-    return login_name
+    login_name
   end
 
   def has_role?(role_sym)
@@ -107,7 +105,7 @@ class Member < ActiveRecord::Base
   end
 
   def current_order
-    orders.where(completed_at: nil).first
+    orders.find_by(completed_at: nil)
   end
 
   # when purchasing a product that gives you a paid account, this method
@@ -128,16 +126,16 @@ class Member < ActiveRecord::Base
 
   def is_paid?
     if account.account_type.is_permanent_paid
-      return true
+      true
     elsif account.account_type.is_paid && account.paid_until >= Time.zone.now
-      return true
+      true
     else
-      return false
+      false
     end
   end
 
   def auth(provider)
-    return authentications.find_by_provider(provider)
+    authentications.find_by(provider: provider)
   end
 
   # Authenticates against Flickr and returns an object we can use for subsequent api calls
@@ -152,14 +150,13 @@ class Member < ActiveRecord::Base
         @flickr.access_secret = flickr_auth.secret
       end
     end
-    return @flickr
+    @flickr
   end
 
   # Fetches a collection of photos from Flickr
   # Returns a [[page of photos], total] pair.
   # Total is needed for pagination.
   def flickr_photos(page_num = 1, set = nil)
-    result = false
     result = if set
                flickr.photosets.getPhotos(
                  photoset_id: set,
@@ -173,11 +170,8 @@ class Member < ActiveRecord::Base
                  per_page: 30
                )
              end
-    if result
-      return [result.photo, result.total]
-    else
-      return [[], 0]
-    end
+    return [result.photo, result.total] if result
+    [[], 0]
   end
 
   # Returns a hash of Flickr photosets' ids and titles
@@ -186,7 +180,7 @@ class Member < ActiveRecord::Base
     flickr.photosets.getList.each do |p|
       sets[p.title] = p.id
     end
-    return sets
+    sets
   end
 
   def interesting?
@@ -194,7 +188,15 @@ class Member < ActiveRecord::Base
     # Member.confirmed.located as those are required for
     # interestingness, as well.
     return true if plantings.present?
-    return false
+    false
+  end
+
+  def Member.login_name_or_email(login)
+    where(["lower(login_name) = :value OR lower(email) = :value", { value: login.downcase }])
+  end
+
+  def Member.case_insensitive_login_name(login)
+    where(["lower(login_name) = :value", { value: login.downcase }])
   end
 
   def Member.interesting
@@ -206,7 +208,7 @@ class Member < ActiveRecord::Base
         interesting_members.push(m)
       end
     end
-    return interesting_members
+    interesting_members
   end
 
   def Member.nearest_to(place)
@@ -217,7 +219,7 @@ class Member < ActiveRecord::Base
         nearby_members = Member.located.sort_by { |x| x.distance_from([latitude, longitude]) }
       end
     end
-    return nearby_members
+    nearby_members
   end
 
   def update_newsletter_subscription
@@ -234,24 +236,22 @@ class Member < ActiveRecord::Base
     end
   end
 
-  def newsletter_subscribe(testing = false)
+  def newsletter_subscribe(gb = Gibbon::API.new, testing = false)
     return true if (Rails.env.test? && !testing)
-    gb = Gibbon::API.new
-    res = gb.lists.subscribe({
-                               id: Growstuff::Application.config.newsletter_list_id,
-                               email: { email: email },
-                               merge_vars: { login_name: login_name },
-                               double_optin: false # they already confirmed their email with us
-                             })
+    gb.lists.subscribe({
+                         id: Growstuff::Application.config.newsletter_list_id,
+                         email: { email: email },
+                         merge_vars: { login_name: login_name },
+                         double_optin: false # they already confirmed their email with us
+                       })
   end
 
-  def newsletter_unsubscribe(testing = false)
+  def newsletter_unsubscribe(gb = Gibbon::API.new, testing = false)
     return true if (Rails.env.test? && !testing)
-    gb = Gibbon::API.new
-    res = gb.lists.unsubscribe({
-                                 id: Growstuff::Application.config.newsletter_list_id,
-                                 email: { email: email }
-                               })
+    gb.lists.unsubscribe({
+                           id: Growstuff::Application.config.newsletter_list_id,
+                           email: { email: email }
+                         })
   end
 
   def already_following?(member)
@@ -259,6 +259,6 @@ class Member < ActiveRecord::Base
   end
 
   def get_follow(member)
-    self.follows.where(followed_id: member.id).first if already_following?(member)
+    self.follows.find_by(followed_id: member.id) if already_following?(member)
   end
 end

app/models/notification.rb

@@ -7,6 +7,7 @@ class Notification < ActiveRecord::Base
 
   default_scope { order('created_at DESC') }
   scope :unread, -> { where(read: false) }
+  scope :by_recipient, ->(recipient) { where(recipient_id: recipient) }
 
   before_create :replace_blank_subject
   after_create :send_email
@@ -16,14 +17,10 @@ class Notification < ActiveRecord::Base
   end
 
   def replace_blank_subject
-    if self.subject.nil? or self.subject =~ /^\s*$/
-      self.subject = "(no subject)"
-    end
+    self.subject = "(no subject)" if self.subject.nil? or self.subject =~ /^\s*$/
   end
 
   def send_email
-    if self.recipient.send_notification_email
-      Notifier.notify(self).deliver_later
-    end
+    Notifier.notify(self).deliver_later if self.recipient.send_notification_email
   end
 end

app/models/order.rb

@@ -12,6 +12,8 @@ class Order < ActiveRecord::Base
 
   before_save :standardize_referral_code
 
+  scope :by_member, ->(member) { where(member: member) }
+
   # total price of an order
   def total
     sum = 0
@@ -19,7 +21,7 @@ class Order < ActiveRecord::Base
       subtotal = i.price * i.quantity
       sum += subtotal
     end
-    return sum
+    sum
   end
 
   # return items in the format ActiveMerchant/PayPal want them
@@ -32,7 +34,7 @@ class Order < ActiveRecord::Base
                    amount: i.price
                  })
     end
-    return items
+    items
   end
 
   # record the paypal details for reference
@@ -54,9 +56,7 @@ class Order < ActiveRecord::Base
   # removes whitespace and forces to uppercase (we're somewhat liberal
   # in what we accept, but we clean it up anyway.)
   def standardize_referral_code
-    if referral_code
-      self.referral_code = referral_code.upcase.gsub /\s/, ''
-    end
+    self.referral_code = referral_code.upcase.gsub /\s/, '' if referral_code
   end
 
   # search orders (used by admin/orders)
@@ -66,22 +66,22 @@ class Order < ActiveRecord::Base
     if args[:for]
       case args[:by]
       when "member"
-        member = Member.find_by_login_name(args[:for])
+        member = Member.find_by(login_name: args[:for])
         if member
           return member.orders
         end
       when "order_id"
-        order = Order.find_by_id(args[:for])
+        order = Order.find_by(id: args[:for])
         if order
           return [order]
         end
       when "paypal_token"
-        order = Order.find_by_paypal_express_token(args[:for])
+        order = Order.find_by(paypal_express_token: args[:for])
         if order
           return [order]
         end
       when "paypal_payer_id"
-        order = Order.find_by_paypal_express_payer_id(args[:for])
+        order = Order.find_by(paypal_express_payer_id: args[:for])
         if order
           return [order]
         end
@@ -90,6 +90,6 @@ class Order < ActiveRecord::Base
         return Order.where(referral_code: args[:for].upcase)
       end
     end
-    return []
+    []
   end
 end

app/models/order_item.rb

@@ -8,8 +8,6 @@ class OrderItem < ActiveRecord::Base
 
   def price_must_be_greater_than_minimum
     @product = Product.find(product_id)
-    if price < @product.min_price
-      errors.add(:price, "must be greater than the product's minimum value")
-    end
+    errors.add(:price, "must be greater than the product's minimum value") if price < @product.min_price
   end
 end

app/models/photo.rb

@@ -1,22 +1,24 @@
 class Photo < ActiveRecord::Base
   belongs_to :owner, class_name: 'Member'
 
-  has_and_belongs_to_many :plantings
-  has_and_belongs_to_many :harvests
-  has_and_belongs_to_many :gardens
-  before_destroy do |photo|
-    photo.plantings.clear
-    photo.harvests.clear
-    photo.gardens.clear
+  Growstuff::Constants::PhotoModels.relations.each do |relation|
+    has_and_belongs_to_many relation.to_sym
   end
 
+  before_destroy { all_associations.clear }
+
   default_scope { order("created_at desc") }
 
-  # remove photos that aren't used by anything
-  def destroy_if_unused
-    unless plantings.size > 0 or harvests.size > 0 or gardens.size > 0
-      self.destroy
+  def all_associations
+    associations = []
+    Growstuff::Constants::PhotoModels.relations.each do |association_name|
+      associations << self.send(association_name.to_s).to_a
     end
+    associations.flatten!
+  end
+
+  def destroy_if_unused
+    self.destroy unless all_associations.size > 0
   end
 
   # This is split into a side-effect free method and a side-effecting method
@@ -26,7 +28,7 @@ class Photo < ActiveRecord::Base
     info = flickr.photos.getInfo(photo_id: flickr_photo_id)
     licenses = flickr.photos.licenses.getInfo()
     license = licenses.find { |l| l.id == info.license }
-    return {
+    {
       title: info.title || "Untitled",
       license_name: license.name,
       license_url: license.url,

app/models/plant_part.rb

@@ -6,7 +6,7 @@ class PlantPart < ActiveRecord::Base
   has_many :crops, -> { uniq }, through: :harvests
 
   def to_s
-    return name
+    name
   end
 
   # Postgres complains if the ORDER BY clause of a SELECT DISTINCT query is
@@ -18,6 +18,6 @@ class PlantPart < ActiveRecord::Base
   # associated to plant parts will not be sorted in the same order as crops
   # on the rest of the site.
   def crops
-    return super.reorder('name')
+    super.reorder('name')
   end
 end

app/models/planting.rb

@@ -1,22 +1,12 @@
 class Planting < ActiveRecord::Base
   extend FriendlyId
+  include PhotoCapable
   friendly_id :planting_slug, use: [:slugged, :finders]
 
   belongs_to :garden
   belongs_to :owner, class_name: 'Member', counter_cache: true
   belongs_to :crop, counter_cache: true
 
-  has_and_belongs_to_many :photos
-
-  before_destroy do |planting|
-    photolist = planting.photos.to_a # save a temp copy of the photo list
-    planting.photos.clear # clear relationship b/w planting and photo
-
-    photolist.each do |photo|
-      photo.destroy_if_unused
-    end
-  end
-
   default_scope { order("created_at desc") }
   scope :finished, -> { where(finished: true) }
   scope :current, -> { where(finished: false) }
@@ -78,7 +68,7 @@ class Planting < ActiveRecord::Base
 
   # location = garden owner + garden name, i.e. "Skud's backyard"
   def location
-    return "#{garden.owner.login_name}'s #{garden}"
+    "#{garden.owner.login_name}'s #{garden}"
   end
 
   # stringify as "beet in Skud's backyard" or similar
@@ -87,11 +77,11 @@ class Planting < ActiveRecord::Base
   end
 
   def default_photo
-    return photos.first
+    photos.first
   end
 
   def interesting?
-    return photos.present?
+    photos.present?
   end
 
   def calculate_days_before_maturity(planting, crop)
@@ -146,6 +136,6 @@ class Planting < ActiveRecord::Base
       interesting_plantings.push(p)
     end
 
-    return interesting_plantings
+    interesting_plantings
   end
 end

app/models/post.rb

@@ -15,12 +15,12 @@ class Post < ActiveRecord::Base
     sender = self.author.id
     self.body.scan(Haml::Filters::GrowstuffMarkdown::MEMBER_REGEX) do |m|
       # find member case-insensitively and add to list of recipients
-      member = Member.where('lower(login_name) = ?', $1.downcase).first
+      member = Member.case_insensitive_login_name($1).first
       recipients << member if member && !recipients.include?(member)
     end
     self.body.scan(Haml::Filters::GrowstuffMarkdown::MEMBER_AT_REGEX) do |m|
       # find member case-insensitively and add to list of recipients
-      member = Member.where('lower(login_name) = ?', $1[1..-1].downcase).first
+      member = Member.case_insensitive_login_name($1[1..-1]).first
       recipients << member if member && !recipients.include?(member)
     end
     # don't send notifications to yourself
@@ -75,7 +75,7 @@ class Post < ActiveRecord::Base
     # look for crops mentioned in the post. eg. [tomato](crop)
     self.body.scan(Haml::Filters::GrowstuffMarkdown::CROP_REGEX) do |m|
       # find crop case-insensitively
-      crop = Crop.where('lower(name) = ?', $1.downcase).first
+      crop = Crop.case_insensitive_name($1).first
       # create association
       self.crops << crop if crop && !self.crops.include?(crop)
     end

app/models/seed.rb

@@ -1,5 +1,6 @@
 class Seed < ActiveRecord::Base
   extend FriendlyId
+  include PhotoCapable
   friendly_id :seed_slug, use: [:slugged, :finders]
 
   belongs_to :crop
@@ -65,9 +66,9 @@ class Seed < ActiveRecord::Base
 
   def tradable?
     if self.tradable_to == 'nowhere'
-      return false
+      false
     else
-      return true
+      true
     end
   end
 
@@ -75,7 +76,7 @@ class Seed < ActiveRecord::Base
     # assuming we're passed something that's already known to be tradable
     # eg. from Seed.tradable scope
     return false if owner.location.blank? # don't want unspecified locations
-    return true
+    true
   end
 
   # Seed.interesting
@@ -91,7 +92,7 @@ class Seed < ActiveRecord::Base
       end
     end
 
-    return interesting_seeds
+    interesting_seeds
   end
 
   def seed_slug

app/validators/approved_validator.rb

@@ -1,7 +1,5 @@
 class ApprovedValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
-    unless record.crop.try(:approved?)
-      record.errors[attribute] << (options[:message] || 'must be approved')
-    end
+    record.errors[attribute] << (options[:message] || 'must be approved') unless record.crop.try(:approved?)
   end
 end

app/views/layouts/_header.html.haml

@@ -4,7 +4,7 @@
   .container
     .navbar-header
       %button.navbar-toggle(data-target="#navbar-collapse" data-toggle="collapse")
-        %span.sr-only Toggle Navigation
+        %span.sr-only= t('.toggle_navigation')
         %span.icon-bar
         %span.icon-bar
         %span.icon-bar
@@ -28,7 +28,7 @@
       %ul.nav.navbar-nav.navbar-right
         %li.dropdown<
           %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => crops_path}
-            Crops
+            = t('.crops')
             %b.caret
           %ul.dropdown-menu
             %li= link_to t('.browse_crops'), crops_path
@@ -37,7 +37,7 @@
             %li= link_to t('.harvests'), harvests_path
         %li.dropdown<
           %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => members_path}
-            Community
+            = t('.community')
             %b.caret
           %ul.dropdown-menu
             %li= link_to t('.community_map'), places_path
@@ -52,7 +52,7 @@
               - if current_member.notifications.unread_count > 0
                 = t('.your_stuff', unread_count: current_member.notifications.unread_count)
               - else
-                #{current_member.login_name}
+                = t('.current_memberlogin_name', :current_memberlogin_name => (current_member.login_name))
               %b.caret
             %ul.dropdown-menu
               %li= link_to t('.profile'), member_path(current_member)
@@ -75,11 +75,11 @@
                   %li= link_to t('.admin'), admin_path
 
 
-          %li= link_to "Sign out", destroy_member_session_path, :method => :delete
+          %li= link_to t('.sign_out'), destroy_member_session_path, :method => :delete
 
         - else
-          %li= link_to 'Sign in', new_member_session_path, :id => 'navbar-signin'
-          %li= link_to 'Sign up', new_member_registration_path, :id => 'navbar-signup'
+          %li= link_to t('.sign_in'), new_member_session_path, :id => 'navbar-signin'
+          %li= link_to t('.sign_up'), new_member_registration_path, :id => 'navbar-signup'
 
 
 - # anchor tag for accessibility link to skip the navigation menu

config.rb

@@ -1,5 +1,5 @@
 # Require any additional compass plugins here.
-
+# rubocop:disable Lint/UselessAssignment
 # Set this to the root of your project when deployed:
 http_path = "/"
 css_dir         = "app/assets/stylesheets"

config/compass.rb

@@ -1,2 +1,3 @@
 # Require any additional compass plugins here.
+# rubocop:disable Lint/UselessAssignment
 project_type = :rails

config/initializers/comfortable_mexican_sofa.rb

@@ -95,8 +95,7 @@ end
 
 module CmsDeviseAuth
   def authenticate
-    unless current_member && current_member.has_role?(:admin)
-      redirect_to root_path, alert: 'Permission denied. Please sign in as an admin user to use the CMS admin area.'
-    end
+    return if current_member && current_member.has_role?(:admin)
+    redirect_to root_path, alert: 'Permission denied. Please sign in as an admin user to use the CMS admin area.'
   end
 end

config/locales/README.md

@@ -0,0 +1,15 @@
+i18n Documentation
+===================
+
+i18n Automation
+-------------
+Automate string extraction from haml into locale files using [haml-i18n-extractor](https://github.com/shaiguitar/haml-i18n-extractor)
+
+```rake i18n:extractor[relative_path_to_view]```
+
+
+####Example
+```rake i18n:extractor[app/views/layouts/_header.html.haml]```
+
+* Creates app/views/layouts/_header.html.i18n-extractor.haml with the expected haml changes to localize app/views/layouts/_header.html.haml. After reviewing the changes, copy app/views/layouts/_header.html.i18n-extractor.haml to app/views/layouts/_header.html.haml
+* Adds new keys to locales/en.yml

config/locales/en.yml

@@ -44,6 +44,9 @@ en:
   gardens:
     form:
       location_helper: If you have a location set in your profile, it will be used when you create a new garden.
+  forums:
+    index:
+      title: Forums
   harvests:
     index:
       title:
@@ -53,7 +56,9 @@ en:
   home:
     blurb:
       already_html: Or %{sign_in} if you already have an account
-      intro: "%{site_name} is a community of food gardeners. We're building an open source platform to help you learn about growing food, track what you plant and harvest, and swap seeds and produce with other gardeners near you."
+      intro: "%{site_name} is a community of food gardeners. We're building an open
+        source platform to help you learn about growing food, track what you plant
+        and harvest, and swap seeds and produce with other gardeners near you."
       perks: Join now for your free garden journal, seed sharing, forums, and more.
       sign_in_linktext: sign in
       sign_up: Sign up
@@ -81,14 +86,24 @@ en:
       api_docs_linktext: API documentation
       buy_account_linktext: buying a paid account
       creative_commons_linktext: Creative Commons license
-      get_involved_body_html: We believe in collaboration, and work closely with our members and the wider food-growing community. Our team includes volunteers from all walks of life and all skill levels. To get involved, visit %{talk_link} or find more information on the %{wiki_link}.
+      get_involved_body_html: We believe in collaboration, and work closely with our
+        members and the wider food-growing community. Our team includes volunteers
+        from all walks of life and all skill levels. To get involved, visit %{talk_link}
+        or find more information on the %{wiki_link}.
       get_involved_title: Get Involved
       github_linktext: Github
-      open_data_body_html: We're building a database of crops, planting advice, seed sources, and other information that anyone can use for free, under a %{creative_commons_link}. You can use this data for research, to build apps, or for any purpose at all.  Read more about our %{wiki_link} and %{api_docs_link}.
+      open_data_body_html: We're building a database of crops, planting advice, seed
+        sources, and other information that anyone can use for free, under a %{creative_commons_link}.
+        You can use this data for research, to build apps, or for any purpose at all.  Read
+        more about our %{wiki_link} and %{api_docs_link}.
       open_data_title: Open Data and APIs
-      open_source_body_html: "%{site_name} is open source software, which means that we share this website's code for free with our community and the world.  We believe that openness, sustainability, and social good go hand in hand.  You can read more about %{why} or check out our code on %{github}."
+      open_source_body_html: "%{site_name} is open source software, which means that
+        we share this website's code for free with our community and the world.  We
+        believe that openness, sustainability, and social good go hand in hand.  You
+        can read more about %{why} or check out our code on %{github}."
       open_source_title: Open Source
-      support_body_html: Growstuff is independent, %{ad_free} and we have no outside investment.  You can support our work by %{buy_account}.
+      support_body_html: Growstuff is independent, %{ad_free} and we have no outside
+        investment.  You can support our work by %{buy_account}.
       support_title: Support Growstuff
       talk_linktext: Growstuff Talk
       why_linktext: why Growstuff is open source
@@ -105,7 +120,8 @@ en:
       view_all: View all seeds
     stats:
       member_linktext: "%{count} members"
-      message_html: So far, %{member} have planted %{number_crops} %{number_plantings} in %{number_gardens}.
+      message_html: So far, %{member} have planted %{number_crops} %{number_plantings}
+        in %{number_gardens}.
       number_crops_linktext: "%{count} crops"
       number_gardens_linktext: "%{count} gardens"
       number_plantings_linktext: "%{count} times"
@@ -129,6 +145,13 @@ en:
       skip: Skip navigation menu
       support_growstuff: Support Growstuff
       your_stuff: Your Stuff (%{unread_count})
+      toggle_navigation: Toggle Navigation
+      crops: Crops
+      community: Community
+      current_memberlogin_name: '"%{current_memberlogin_name}"'
+      sign_out: Sign out
+      sign_in: Sign in
+      sign_up: Sign up
   members:
     index:
       title: "%{site_name} members"

config/locales/ja.yml

@@ -1,3 +1,4 @@
+---
 ja:
   home:
     blurb:

config/setup_load_paths.rb

@@ -1,7 +1,5 @@
 if ENV['MY_RUBY_HOME'] && ENV['MY_RUBY_HOME'].include?('rvm')
   begin
-    rvm_path     = File.dirname(File.dirname(ENV['MY_RUBY_HOME']))
-    rvm_lib_path = File.join(rvm_path, 'lib')
     require 'rvm'
     RVM.use_from_path! File.dirname(File.dirname(__FILE__))
   rescue LoadError

db/migrate/20150201052245_create_cms.rb

@@ -1,5 +1,5 @@
 class CreateCms < ActiveRecord::Migration
-  def self.up # rubocop:disable Metrics/MethodLength
+  def self.up # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
     text_limit = case ActiveRecord::Base.connection.adapter_name
                  when 'PostgreSQL'
                    {}

db/migrate/20161201154922_add_photos_seeds_table.rb

@@ -0,0 +1,9 @@
+class AddPhotosSeedsTable < ActiveRecord::Migration
+  def change
+    create_table :photos_seeds, id: false do |t|
+      t.integer :photo_id
+      t.integer :seed_id
+    end
+    add_index(:photos_seeds, [:seed_id, :photo_id])
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20161129021533) do
+ActiveRecord::Schema.define(version: 20161201154922) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -373,6 +373,13 @@ ActiveRecord::Schema.define(version: 20161129021533) do
     t.integer "planting_id"
   end
 
+  create_table "photos_seeds", id: false, force: :cascade do |t|
+    t.integer "photo_id"
+    t.integer "seed_id"
+  end
+
+  add_index "photos_seeds", ["seed_id", "photo_id"], name: "index_photos_seeds_on_seed_id_and_photo_id", using: :btree
+
   create_table "plant_parts", force: :cascade do |t|
     t.string   "name"
     t.datetime "created_at"

db/seeds.rb

@@ -87,7 +87,7 @@ def load_test_users
       suburb_file.pos = 0 if suburb_file.eof?
       row = CSV.parse(suburb_file.readline)
 
-      suburb, country, state, latitude, longitude = row[0]
+      suburb, _country, _state, latitude, longitude = row[0]
       # Using 'update_column' method instead of 'update' so that
       # it avoids accessing Geocoding service for faster processing
       @user.gardens.first.update_columns(location: suburb, latitude: latitude, longitude: longitude)
@@ -141,7 +141,7 @@ def create_cropbot
   @cropbot_user.skip_confirmation!
   @cropbot_user.roles << @wrangler
   @cropbot_user.save!
-  @cropbot_user.account.account_type = AccountType.find_by_name("Staff")
+  @cropbot_user.account.account_type = AccountType.find_by(name: "Staff")
   @cropbot_user.account.save
 end
 

lib/geocodable.rb

@@ -5,17 +5,9 @@ module Geocodable
 
   private
 
-  def geocode
-    unless self.location.blank?
-      self.latitude, self.longitude =
-        Geocoder.coordinates(location, params: { limit: 1 })
-    end
-  end
-
   def empty_unwanted_geocodes
-    if self.location.blank?
-      self.latitude = nil
-      self.longitude = nil
-    end
+    return unless self.location.blank?
+    self.latitude = nil
+    self.longitude = nil
   end
 end

lib/haml/filters/escaped_markdown.rb

@@ -5,7 +5,7 @@ module Haml::Filters
   module EscapedMarkdown
     include Haml::Filters::Base
     def render(text)
-      return Haml::Helpers.html_escape Haml::Filters::GrowstuffMarkdown.render(text)
+      Haml::Helpers.html_escape Haml::Filters::GrowstuffMarkdown.render(text)
     end
   end
 

lib/haml/filters/growstuff_markdown.rb

@@ -26,7 +26,7 @@ module Haml::Filters
       expanded = expanded.gsub(MEMBER_REGEX) do |m|
         member_str = $1
         # find member case-insensitively
-        member = Member.where('lower(login_name) = ?', member_str.downcase).first
+        member = Member.case_insensitive_login_name(member_str).first
         if member
           url = Rails.application.routes.url_helpers.member_url(member, only_path: true)
           "[#{member_str}](#{url})"
@@ -39,7 +39,7 @@ module Haml::Filters
       expanded = expanded.gsub(MEMBER_AT_REGEX) do |m|
         member_str = $1
         # find member case-insensitively
-        member = Member.where('lower(login_name) = ?', member_str[1..-1].downcase).first
+        member = Member.case_insensitive_login_name(member_str[1..-1]).first
         if member
           url = Rails.application.routes.url_helpers.member_url(member, only_path: true)
           "[#{member_str}](#{url})"
@@ -50,7 +50,7 @@ module Haml::Filters
 
       expanded = expanded.gsub(MEMBER_ESCAPE_AT_REGEX, "")
 
-      return BlueCloth.new(expanded).to_html
+      BlueCloth.new(expanded).to_html
     end
   end
 

lib/tasks/growstuff.rake

@@ -146,7 +146,7 @@ namespace :growstuff do
       end
 
       puts "Making Skud a staff account..."
-      @skud = Member.find_by_login_name('Skud')
+      @skud = Member.find_by(login_name: 'Skud')
       if @skud
         @skud.account.account_type = @staff_account
         @skud.account.save
@@ -157,7 +157,7 @@ namespace :growstuff do
 
     desc "June 2013: replace nil account_types with free accounts"
     task nil_account_type: :environment do
-      free = AccountType.find_by_name("Free")
+      free = AccountType.find_by(name: "Free")
       raise "Free account type not found: run rake growstuff:oneoff:setup_shop"\
         unless free
       Account.all.each do |a|
@@ -187,9 +187,9 @@ namespace :growstuff do
 
     desc "August 2013: set default creator on existing crops"
     task set_default_crop_creator: :environment do
-      cropbot = Member.find_by_login_name("cropbot")
+      cropbot = Member.find_by(login_name: "cropbot")
       raise "cropbot not found: create cropbot member on site or run rake db:seed" unless cropbot
-      cropbot.account.account_type = AccountType.find_by_name("Staff") # set this just because it's nice
+      cropbot.account.account_type = AccountType.find_by(name: "Staff") # set this just because it's nice
       cropbot.account.save
       Crop.find_each do |crop|
         unless crop.creator
@@ -294,13 +294,11 @@ namespace :growstuff do
       require 'csv'
       file = "db/seeds/alternate_names_201410.csv"
       puts "Loading alternate names from #{file}..."
-      cropbot = Member.find_by_login_name("cropbot")
+      cropbot = Member.find_by(login_name: "cropbot")
       CSV.foreach(file) do |row|
-        crop_id, crop_name, alternate_names = row
-        if alternate_names.blank? then
-          next
-        end
-        crop = Crop.find_by_name(crop_name)
+        _crop_id, crop_name, alternate_names = row
+        next if alternate_names.blank?
+        crop = Crop.find_by(name: crop_name)
         if crop
           alternate_names.split(/,\s*/).each do |an|
             AlternateName.where(

lib/tasks/i18n.rake

@@ -0,0 +1,18 @@
+namespace :i18n do
+  desc "sort all i18n locale keys"
+  task :normalize do
+    `i18n-tasks normalize`
+  end
+
+  desc "translate haml strings into i18 en locale using haml-i18n-extractor"
+  task :extractor, [:haml_path] do |_t, args|
+    require 'haml-i18n-extractor'
+    haml_path = args[:haml_path]
+    begin
+      translate = Haml::I18n::Extractor.new(haml_path)
+      translate.run
+    rescue Haml::I18n::Extractor::InvalidSyntax
+      puts "There was an error with #{haml_path}"
+    end
+  end
+end

spec/controllers/accounts_controller_spec.rb

@@ -25,6 +25,6 @@ describe AccountsController do
     # allowed. This method has been left here in case it's useful in
     # future.
     member = FactoryGirl.create(:member)
-    return member.account
+    member.account
   end
 end

spec/controllers/admin/orders_controller_spec.rb

@@ -23,7 +23,6 @@ describe Admin::OrdersController do
     end
 
     it "sets an error message if nothing found" do
-      order = FactoryGirl.create(:order)
       get :search, { search_by: 'order_id', search_text: 'foo' }
       flash[:alert].should match /Couldn't find order with/
     end

spec/controllers/roles_controller_spec.rb

@@ -24,7 +24,7 @@ describe RolesController do
       role = Role.create! valid_attributes
       get :index, {}
       # note that admin role exists because of login_admin_member
-      assigns(:roles).should eq([Role.find_by_name('admin'), role])
+      assigns(:roles).should eq([Role.find_by(name: 'admin'), role])
     end
   end
 end

spec/factories/member.rb

@@ -49,13 +49,13 @@ FactoryGirl.define do
     factory :edinburgh_member do
       location 'Edinburgh'
       latitude 55.953252
-      longitude -3.188267
+      longitude { -3.188267 }
     end
 
     factory :south_pole_member do
       sequence(:login_name) { |n| "ScottRF#{n}" }
       location 'Amundsen-Scott Base, Antarctica'
-      latitude -90
+      latitude { -90 }
       longitude 0
     end
 

spec/features/harvests/browse_harvests_spec.rb

@@ -11,7 +11,7 @@ feature "browse harvests" do
   feature 'blank optional fields' do
     let!(:harvest) { create :harvest, :no_description }
 
-    before (:each) do
+    before(:each) do
       visit harvests_path
     end
 
@@ -23,7 +23,7 @@ feature "browse harvests" do
   feature "filled in optional fields" do
     let!(:harvest) { create :harvest, :long_description }
 
-    before (:each) do
+    before(:each) do
       visit harvests_path
     end
 

spec/features/plantings/planting_a_crop_spec.rb

@@ -59,7 +59,7 @@ feature "Planting a crop", :js do
 
       @a_past_date = 15.days.ago.strftime("%Y-%m-%d")
       @right_now = Date.today.strftime("%Y-%m-%d")
-      @a_future_date = 1.years.from_now.strftime("%Y-%m-%d")
+      @a_future_date = 1.year.from_now.strftime("%Y-%m-%d")
     end
 
     it "should show that it is not planted yet" do

spec/features/signin_spec.rb

@@ -6,6 +6,15 @@ feature "signin", js: true do
   let(:wrangler) { create :crop_wrangling_member }
   let(:notification) { create :notification }
 
+  scenario "via email address" do
+    visit crops_path # some random page
+    click_link 'Sign in'
+    fill_in 'Login', with: member.email
+    fill_in 'Password', with: member.password
+    click_button 'Sign in'
+    expect(page).to have_content("Sign out")
+  end
+
   scenario "redirect to previous page after signin" do
     visit crops_path # some random page
     click_link 'Sign in'
@@ -66,7 +75,7 @@ feature "signin", js: true do
       # Ordinarily done by database_cleaner
       Member.where(login_name: 'tdawg').delete_all
 
-      member = create :member, login_name: 'tdawg', email: 'example.oauth.facebook@example.com'
+      create :member, login_name: 'tdawg', email: 'example.oauth.facebook@example.com'
 
       # Start the test
       visit root_path

spec/features/unsubscribing_spec.rb

@@ -52,7 +52,6 @@ feature "unsubscribe" do
     # visit /members/unsubscribe/somestring ie.parameter to the URL is a random string
     visit unsubscribe_member_url("type=send_planting_reminder&member_id=#{member.id}")
     expect(page).to have_content "We're sorry, there was an error"
-    updated_member = Member.find(member.id) # reload the member
     expect(member.send_planting_reminder).to eq(true)
     expect(member.send_notification_email).to eq(true)
   end

spec/helpers/gardens_helper_spec.rb

@@ -23,7 +23,6 @@ describe GardensHelper do
         description: 'a' * 130
       )
       result = helper.display_garden_description(garden)
-      link = link_to("Read more", garden_path(garden))
       expect(result).to eq 'a' * 130
     end
 
@@ -38,17 +37,13 @@ describe GardensHelper do
 
   describe "garden plantings" do
     it "is missing" do
-      garden = FactoryGirl.create(:garden)
-      plantings = nil
-      result = helper.display_garden_plantings(plantings)
+      result = helper.display_garden_plantings(nil)
       expect(result).to eq "None"
     end
 
     it "has 1 planting" do
-      garden = FactoryGirl.create(:garden)
-      plantings = []
       crop = FactoryGirl.create(:crop)
-      plantings << FactoryGirl.create(:planting, quantity: 10, crop: crop)
+      plantings = [FactoryGirl.create(:planting, quantity: 10, crop: crop)]
       result = helper.display_garden_plantings(plantings)
 
       output = "<li>"
@@ -59,7 +54,6 @@ describe GardensHelper do
     end
 
     it "has 2 plantings" do
-      garden = FactoryGirl.create(:garden)
       plantings = []
 
       crop1 = FactoryGirl.create(:crop)
@@ -82,7 +76,6 @@ describe GardensHelper do
     end
 
     it "has 3 plantings" do
-      garden = FactoryGirl.create(:garden)
       plantings = []
 
       crop1 = FactoryGirl.create(:crop)

spec/helpers/notifications_helper_spec.rb

@@ -6,8 +6,6 @@ describe NotificationsHelper do
 
     it "replies to PMs with PMs" do
       notification = FactoryGirl.create(:notification, recipient_id: member.id, post_id: nil)
-      subject = "Re: " + notification.subject
-
       link = helper.reply_link(notification)
       link.should_not be_nil
       link.should eq reply_notification_url(notification)

spec/helpers/plantings_helper_spec.rb

@@ -1,6 +1,69 @@
 require 'rails_helper'
 
 describe PlantingsHelper do
+  describe "display_days_before_maturity" do
+    it "handles nil planted_at, nil finished_at, non-nil days_until_maturity" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: nil,
+        finished_at: nil,
+        days_before_maturity: 17
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "unknown"
+    end
+
+    it "handles non-nil planted_at and d_b_m, nil finished_at" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.current,
+        finished_at: nil,
+        days_before_maturity: 17
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "17"
+    end
+
+    it "handles completed plantings" do
+      planting = FactoryGirl.build(:planting, finished: true)
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "0"
+    end
+
+    it "handles plantings that should have finished" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.new(0, 1, 1),
+        finished_at: nil,
+        days_before_maturity: "17"
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "0"
+    end
+
+    it "handles nil d_b_m and nil finished_at" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.new(2012, 5, 12),
+        finished_at: nil,
+        days_before_maturity: nil
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "unknown"
+    end
+
+    it "handles finished_at dates in the future" do
+      planting = FactoryGirl.build(:planting,
+        quantity: 5,
+        planted_at: Date.current,
+        finished_at: Date.current + 5,
+        days_before_maturity: nil
+      )
+      result = helper.display_days_before_maturity(planting)
+      expect(result).to eq "5"
+    end
+  end
+
   describe "display_planting" do
     let!(:member) { FactoryGirl.build(:member, login_name: 'crop_lady') }
 

spec/helpers/seeds_helper_spec.rb

@@ -23,7 +23,6 @@ describe SeedsHelper do
         description: 'a' * 130
       )
       result = helper.display_seed_description(seed)
-      link = link_to("Read more", seed_path(seed))
       expect(result).to eq 'a' * 130
     end
 

spec/lib/haml/filters/growstuff_markdown_spec.rb

@@ -3,29 +3,23 @@ require 'haml/filters'
 require 'haml/filters/growstuff_markdown'
 
 def input_link(name)
-  return "[#{name}](crop)"
+  "[#{name}](crop)"
 end
 
 def output_link(crop, name = nil)
   url = Rails.application.routes.url_helpers.crop_url(crop, host: Growstuff::Application.config.host)
-  if name
-    return "<a href=\"#{url}\">#{name}</a>"
-  else
-    return "<a href=\"#{url}\">#{crop.name}</a>"
-  end
+  return "<a href=\"#{url}\">#{name}</a>" if name
+  "<a href=\"#{url}\">#{crop.name}</a>"
 end
 
 def input_member_link(name)
-  return "[#{name}](member)"
+  "[#{name}](member)"
 end
 
 def output_member_link(member, name = nil)
   url = Rails.application.routes.url_helpers.member_url(member, only_path: true)
-  if name
-    return "<a href=\"#{url}\">#{name}</a>"
-  else
-    return "<a href=\"#{url}\">#{member.login_name}</a>"
-  end
+  return "<a href=\"#{url}\">#{name}</a>" if name
+  "<a href=\"#{url}\">#{member.login_name}</a>"
 end
 
 describe 'Haml::Filters::Growstuff_Markdown' do

spec/models/crop_spec.rb

@@ -10,7 +10,7 @@ describe Crop do
 
     it 'should be fetchable from the database' do
       crop.save
-      @crop2 = Crop.find_by_name('tomato')
+      @crop2 = Crop.find_by(name: 'tomato')
       @crop2.en_wikipedia_url.should == "http://en.wikipedia.org/wiki/Tomato"
       @crop2.slug.should == "tomato"
     end
@@ -189,25 +189,25 @@ describe Crop do
     let(:crop) { FactoryGirl.create(:tomato) }
 
     it 'returns a hash of sunniness values' do
-      planting1 = FactoryGirl.create(:sunny_planting, crop: crop)
-      planting2 = FactoryGirl.create(:sunny_planting, crop: crop)
-      planting3 = FactoryGirl.create(:semi_shady_planting, crop: crop)
-      planting4 = FactoryGirl.create(:shady_planting, crop: crop)
+      FactoryGirl.create(:sunny_planting, crop: crop)
+      FactoryGirl.create(:sunny_planting, crop: crop)
+      FactoryGirl.create(:semi_shady_planting, crop: crop)
+      FactoryGirl.create(:shady_planting, crop: crop)
       crop.sunniness.should be_an_instance_of Hash
     end
 
     it 'counts each sunniness value' do
-      planting1 = FactoryGirl.create(:sunny_planting, crop: crop)
-      planting2 = FactoryGirl.create(:sunny_planting, crop: crop)
-      planting3 = FactoryGirl.create(:semi_shady_planting, crop: crop)
-      planting4 = FactoryGirl.create(:shady_planting, crop: crop)
+      FactoryGirl.create(:sunny_planting, crop: crop)
+      FactoryGirl.create(:sunny_planting, crop: crop)
+      FactoryGirl.create(:semi_shady_planting, crop: crop)
+      FactoryGirl.create(:shady_planting, crop: crop)
       crop.sunniness.should == { 'sun' => 2, 'shade' => 1, 'semi-shade' => 1 }
     end
 
     it 'ignores unused sunniness values' do
-      planting1 = FactoryGirl.create(:sunny_planting, crop: crop)
-      planting2 = FactoryGirl.create(:sunny_planting, crop: crop)
-      planting3 = FactoryGirl.create(:semi_shady_planting, crop: crop)
+      FactoryGirl.create(:sunny_planting, crop: crop)
+      FactoryGirl.create(:sunny_planting, crop: crop)
+      FactoryGirl.create(:semi_shady_planting, crop: crop)
       crop.sunniness.should == { 'sun' => 2, 'semi-shade' => 1 }
     end
   end
@@ -216,25 +216,25 @@ describe Crop do
     let(:crop) { FactoryGirl.create(:tomato) }
 
     it 'returns a hash of sunniness values' do
-      planting1 = FactoryGirl.create(:seed_planting, crop: crop)
-      planting2 = FactoryGirl.create(:seed_planting, crop: crop)
-      planting3 = FactoryGirl.create(:seedling_planting, crop: crop)
-      planting4 = FactoryGirl.create(:cutting_planting, crop: crop)
+      FactoryGirl.create(:seed_planting, crop: crop)
+      FactoryGirl.create(:seed_planting, crop: crop)
+      FactoryGirl.create(:seedling_planting, crop: crop)
+      FactoryGirl.create(:cutting_planting, crop: crop)
       crop.planted_from.should be_an_instance_of Hash
     end
 
     it 'counts each planted_from value' do
-      planting1 = FactoryGirl.create(:seed_planting, crop: crop)
-      planting2 = FactoryGirl.create(:seed_planting, crop: crop)
-      planting3 = FactoryGirl.create(:seedling_planting, crop: crop)
-      planting4 = FactoryGirl.create(:cutting_planting, crop: crop)
+      FactoryGirl.create(:seed_planting, crop: crop)
+      FactoryGirl.create(:seed_planting, crop: crop)
+      FactoryGirl.create(:seedling_planting, crop: crop)
+      FactoryGirl.create(:cutting_planting, crop: crop)
       crop.planted_from.should == { 'seed' => 2, 'seedling' => 1, 'cutting' => 1 }
     end
 
     it 'ignores unused planted_from values' do
-      planting1 = FactoryGirl.create(:seed_planting, crop: crop)
-      planting2 = FactoryGirl.create(:seed_planting, crop: crop)
-      planting3 = FactoryGirl.create(:seedling_planting, crop: crop)
+      FactoryGirl.create(:seed_planting, crop: crop)
+      FactoryGirl.create(:seed_planting, crop: crop)
+      FactoryGirl.create(:seedling_planting, crop: crop)
       crop.planted_from.should == { 'seed' => 2, 'seedling' => 1 }
     end
   end
@@ -578,6 +578,17 @@ describe Crop do
       expect(loaded.parent).to eq parent
     end
 
+    it "loads a crop with a missing parent" do
+      tomato_row = "tomato,http://en.wikipedia.org/wiki/Tomato,parent"
+
+      CSV.parse(tomato_row) do |row|
+        Crop.create_from_csv(row)
+      end
+
+      loaded = Crop.last
+      expect(loaded.parent).to be_nil
+    end
+
     it "doesn't add unnecessary duplicate crops" do
       tomato_row = "tomato,http://en.wikipedia.org/wiki/Tomato,,Solanum lycopersicum"
 

spec/models/follow_spec.rb

@@ -20,7 +20,7 @@ describe Follow do
   end
 
   context "when follow is created" do
-    before (:each) do
+    before(:each) do
       @follow = Follow.create(follower_id: @member1.id, followed_id: @member2.id)
     end
 

spec/models/garden_spec.rb

@@ -90,7 +90,7 @@ describe Garden do
 
   context 'ordering' do
     it "should be sorted alphabetically" do
-      z = FactoryGirl.create(:garden_z)
+      FactoryGirl.create(:garden_z)
       a = FactoryGirl.create(:garden_a)
       Garden.first.should == a
     end

spec/models/member_spec.rb

@@ -5,10 +5,10 @@ describe 'member' do
     let(:member) { FactoryGirl.create(:member) }
 
     it 'should be fetchable from the database' do
-      @member2 = Member.find(member.id)
-      @member2.should be_an_instance_of Member
-      @member2.login_name.should match(/member\d+/)
-      @member2.encrypted_password.should_not be_nil
+      member2 = Member.find(member.id)
+      member2.should be_an_instance_of Member
+      member2.login_name.should match(/member\d+/)
+      member2.encrypted_password.should_not be_nil
     end
 
     it 'should have a friendly slug' do
@@ -43,8 +43,8 @@ describe 'member' do
     end
 
     it 'should be able to fetch posts' do
-      @post = FactoryGirl.create(:post, author: member)
-      member.posts.should eq [@post]
+      post = FactoryGirl.create(:post, author: member)
+      member.posts.should eq [post]
     end
 
     it 'should be able to fetch gardens' do
@@ -52,19 +52,19 @@ describe 'member' do
     end
 
     it 'has many plantings' do
-      @planting = FactoryGirl.create(:planting, owner: member)
+      FactoryGirl.create(:planting, owner: member)
       member.plantings.size.should eq 1
     end
 
     it "has many comments" do
-      @comment1 = FactoryGirl.create(:comment, author: member)
-      @comment2 = FactoryGirl.create(:comment, author: member)
+      FactoryGirl.create(:comment, author: member)
+      FactoryGirl.create(:comment, author: member)
       member.comments.size.should == 2
     end
 
     it "has many forums" do
-      @forum1 = FactoryGirl.create(:forum, owner: member)
-      @forum2 = FactoryGirl.create(:forum, owner: member)
+      FactoryGirl.create(:forum, owner: member)
+      FactoryGirl.create(:forum, owner: member)
       member.forums.size.should == 2
     end
 
@@ -101,10 +101,10 @@ describe 'member' do
 
   context 'newsletter scope' do
     it 'finds newsletter recipients' do
-      @member1 = FactoryGirl.create(:member)
-      @member2 = FactoryGirl.create(:newsletter_recipient_member)
-      Member.wants_newsletter.should include @member2
-      Member.wants_newsletter.should_not include @member1
+      member1 = FactoryGirl.create(:member)
+      member2 = FactoryGirl.create(:newsletter_recipient_member)
+      Member.wants_newsletter.should include member2
+      Member.wants_newsletter.should_not include member1
     end
   end
 
@@ -126,17 +126,18 @@ describe 'member' do
 
   context 'case sensitivity' do
     it 'preserves case of login name' do
-      member = FactoryGirl.create(:member, login_name: "BOB")
-      check = Member.find('bob')
-      check.login_name.should eq 'BOB'
+      FactoryGirl.create(:member, login_name: "BOB")
+      Member.find('bob').login_name.should eq 'BOB'
     end
   end
 
   context 'ordering' do
+    before do
+      FactoryGirl.create(:member, login_name: "Zoe")
+      FactoryGirl.create(:member, login_name: "Anna")
+    end
     it "should be sorted by name" do
-      z = FactoryGirl.create(:member, login_name: "Zoe")
-      a = FactoryGirl.create(:member, login_name: "Anna")
-      Member.first.should == a
+      expect(Member.first.login_name).to eq("Anna")
     end
   end
 
@@ -197,22 +198,22 @@ describe 'member' do
     end
 
     it 'sets up roles in factories' do
-      @admin = FactoryGirl.create(:admin_member)
-      @admin.has_role?(:admin).should eq true
+      admin = FactoryGirl.create(:admin_member)
+      admin.has_role?(:admin).should eq true
     end
 
     it 'converts role names properly' do
       # need to make sure spaces get turned to underscores
-      @role = FactoryGirl.create(:role, name: "a b c")
-      member.roles << @role
+      role = FactoryGirl.create(:role, name: "a b c")
+      member.roles << role
       member.has_role?(:a_b_c).should eq true
     end
   end
 
   context 'confirmed scope' do
     before(:each) do
-      @member1 = FactoryGirl.create(:member)
-      @member2 = FactoryGirl.create(:member)
+      FactoryGirl.create(:member)
+      FactoryGirl.create(:member)
     end
 
     it 'sees confirmed members' do
@@ -220,7 +221,7 @@ describe 'member' do
     end
 
     it 'ignores unconfirmed members' do
-      @member3 = FactoryGirl.create(:unconfirmed_member)
+      FactoryGirl.create(:unconfirmed_member)
       Member.confirmed.size.should == 2
     end
   end
@@ -228,29 +229,29 @@ describe 'member' do
   context 'located scope' do
     # located members must have location, lat, long
     it 'finds members who have locations' do
-      @london_member = FactoryGirl.create(:london_member)
-      Member.located.should include @london_member
+      london_member = FactoryGirl.create(:london_member)
+      Member.located.should include london_member
     end
 
     it 'ignores members with blank locations' do
-      @nowhere_member = FactoryGirl.create(:member)
-      Member.located.should_not include @nowhere_member
+      nowhere_member = FactoryGirl.create(:member)
+      Member.located.should_not include nowhere_member
     end
 
     it 'ignores members with blank lat/long' do
-      @london_member = FactoryGirl.create(:london_member)
-      @london_member.latitude = nil
-      @london_member.longitude = nil
-      @london_member.save(validate: false)
-      Member.located.should_not include @london_member
+      london_member = FactoryGirl.create(:london_member)
+      london_member.latitude = nil
+      london_member.longitude = nil
+      london_member.save(validate: false)
+      Member.located.should_not include london_member
     end
   end
 
   context 'near location' do
     it 'finds nearby members and sorts them' do
-      @edinburgh_member = FactoryGirl.create(:edinburgh_member)
-      @london_member = FactoryGirl.create(:london_member)
-      Member.nearest_to('Greenwich, UK').should eq [@london_member, @edinburgh_member]
+      edinburgh_member = FactoryGirl.create(:edinburgh_member)
+      london_member = FactoryGirl.create(:london_member)
+      Member.nearest_to('Greenwich, UK').should eq [london_member, edinburgh_member]
     end
   end
 
@@ -262,36 +263,38 @@ describe 'member' do
     # 4) ordered by the most recent sign in
 
     it 'finds interesting members' do
-      @member1 = FactoryGirl.create(:london_member)
-      @member2 = FactoryGirl.create(:london_member)
-      @member3 = FactoryGirl.create(:london_member)
-      @member4 = FactoryGirl.create(:unconfirmed_member)
+      members = [
+        :london_member, :london_member, :london_member,
+        :unconfirmed_member, # !1
+        :london_member,      # 1, 2, !3
+        :member              # 1, !2, 3
+      ].collect { |m| FactoryGirl.create(m) }
 
-      [@member1, @member2, @member3, @member4].each do |m|
-        FactoryGirl.create(:planting, owner: m)
+      [0, 1, 2, 3, 5].each do |i|
+        FactoryGirl.create(:planting, owner: members[i])
       end
 
-      @member1.updated_at = 3.days.ago
-      @member2.updated_at = 2.days.ago
-      @member3.updated_at = 1.days.ago
+      members[0].updated_at = 3.days.ago
+      members[1].updated_at = 2.days.ago
+      members[2].updated_at = 1.day.ago
 
-      Member.interesting.should eq [@member3, @member2, @member1]
+      Member.interesting.should eq [members[2], members[1], members[0]]
     end
   end
 
   context 'orders' do
     it 'finds the current order' do
-      @member = FactoryGirl.create(:member)
-      @order1 = FactoryGirl.create(:completed_order, member: @member)
-      @order2 = FactoryGirl.create(:order, member: @member)
-      @member.current_order.should eq @order2
+      member = FactoryGirl.create(:member)
+      FactoryGirl.create(:completed_order, member: member)
+      order2 = FactoryGirl.create(:order, member: member)
+      member.current_order.should eq order2
     end
 
     it "copes if there's no current order" do
-      @member = FactoryGirl.create(:member)
-      @order1 = FactoryGirl.create(:completed_order, member: @member)
-      @order2 = FactoryGirl.create(:completed_order, member: @member)
-      @member.current_order.should be_nil
+      member = FactoryGirl.create(:member)
+      FactoryGirl.create(:completed_order, member: member)
+      FactoryGirl.create(:completed_order, member: member)
+      member.current_order.should be_nil
     end
   end
 
@@ -299,39 +302,39 @@ describe 'member' do
     let(:member) { FactoryGirl.create(:member) }
 
     it "recognises a permanent paid account" do
-      @account_type = FactoryGirl.create(:account_type,
+      account_type = FactoryGirl.create(:account_type,
         is_paid: true, is_permanent_paid: true)
-      member.account.account_type = @account_type
+      member.account.account_type = account_type
       member.is_paid?.should be(true)
     end
 
     it "recognises a current paid account" do
-      @account_type = FactoryGirl.create(:account_type,
+      account_type = FactoryGirl.create(:account_type,
         is_paid: true, is_permanent_paid: false)
-      member.account.account_type = @account_type
+      member.account.account_type = account_type
       member.account.paid_until = Time.zone.now + 1.month
       member.is_paid?.should be(true)
     end
 
     it "recognises an expired paid account" do
-      @account_type = FactoryGirl.create(:account_type,
+      account_type = FactoryGirl.create(:account_type,
         is_paid: true, is_permanent_paid: false)
-      member.account.account_type = @account_type
+      member.account.account_type = account_type
       member.account.paid_until = Time.zone.now - 1.minute
       member.is_paid?.should be(false)
     end
 
     it "recognises a free account" do
-      @account_type = FactoryGirl.create(:account_type,
+      account_type = FactoryGirl.create(:account_type,
         is_paid: false, is_permanent_paid: false)
-      member.account.account_type = @account_type
+      member.account.account_type = account_type
       member.is_paid?.should be(false)
     end
 
     it "recognises a free account even with paid_until set" do
-      @account_type = FactoryGirl.create(:account_type,
+      account_type = FactoryGirl.create(:account_type,
         is_paid: false, is_permanent_paid: false)
-      member.account.account_type = @account_type
+      member.account.account_type = account_type
       member.account.paid_until = Time.zone.now + 1.month
       member.is_paid?.should be(false)
     end
@@ -353,11 +356,11 @@ describe 'member' do
       member.update_account_after_purchase(product)
 
       # stringify to avoid millisecond problems...
-      member.account.paid_until.to_s.should eq (Time.zone.now + 3.months).to_s
+      member.account.paid_until.to_s.should eq((Time.zone.now + 3.months).to_s)
 
       # and again to make sure it works for currently paid accounts
       member.update_account_after_purchase(product)
-      member.account.paid_until.to_s.should eq (Time.zone.now + 3.months + 3.months).to_s
+      member.account.paid_until.to_s.should eq((Time.zone.now + 3.months + 3.months).to_s)
     end
   end
 
@@ -398,4 +401,19 @@ describe 'member' do
       end
     end
   end
+
+  context 'subscriptions' do
+    let(:member) { FactoryGirl.create(:member) }
+    let(:gb) { instance_double("Gibbon::API.new") }
+
+    it 'subscribes to the newsletter' do
+      expect(gb).to receive_message_chain('lists.subscribe')
+      member.newsletter_subscribe(gb, true)
+    end
+
+    it 'unsubscribes from the newsletter' do
+      expect(gb).to receive_message_chain('lists.unsubscribe')
+      member.newsletter_unsubscribe(gb, true)
+    end
+  end
 end

spec/models/order_spec.rb

@@ -8,6 +8,19 @@ describe Order do
       order_id: @order.id, product_id: @product.id)
   end
 
+  describe '#by_member_id' do
+    before do
+      @member1 = FactoryGirl.create(:member)
+      @member2 = FactoryGirl.create(:member)
+      @order1 = Order.create!(member_id: @member1.id)
+      @order2 = Order.create!(member_id: @member2.id)
+    end
+
+    it "only returns orders belonging to member" do
+      Order.by_member(@member1).should eq [@order1]
+    end
+  end
+
   it 'has order_items' do
     @order.order_items.first.should eq @order_item
   end

spec/models/photo_spec.rb

@@ -85,11 +85,13 @@ describe Photo do
 
         planting.destroy # photo is still used by harvest and garden
         photo.reload
+
         expect(photo.plantings.size).to eq 0
         expect(photo.harvests.size).to eq 1
 
         harvest.destroy
         garden.destroy # photo is now no longer used by anything
+        photo.reload
 
         expect(photo.plantings.size).to eq 0
         expect(photo.harvests.size).to eq 0

spec/models/post_spec.rb

@@ -18,50 +18,57 @@ describe Post do
   end
 
   it "should have a slug" do
-    @post = FactoryGirl.create(:post, author: member)
-    @time = @post.created_at
-    @datestr = @time.strftime("%Y%m%d")
+    post = FactoryGirl.create(:post, author: member)
+    time = post.created_at
+    datestr = time.strftime("%Y%m%d")
     # 2 digit day and month, full-length years
     # Counting digits using Math.log is not precise enough!
-    @datestr.size.should == 4 + @time.year.to_s.size
-    @post.slug.should == "#{member.login_name}-#{@datestr}-a-post"
+    datestr.size.should == 4 + time.year.to_s.size
+    post.slug.should == "#{member.login_name}-#{datestr}-a-post"
   end
 
   it "has many comments" do
-    @post = FactoryGirl.create(:post, author: member)
-    @comment1 = FactoryGirl.create(:comment, post: @post)
-    @comment2 = FactoryGirl.create(:comment, post: @post)
-    @post.comments.size.should == 2
+    post = FactoryGirl.create(:post, author: member)
+    FactoryGirl.create(:comment, post: post)
+    FactoryGirl.create(:comment, post: post)
+    post.comments.size.should == 2
+  end
+
+  it "supports counting comments" do
+    post = FactoryGirl.create(:post, author: member)
+    FactoryGirl.create(:comment, post: post)
+    FactoryGirl.create(:comment, post: post)
+    post.comment_count.should == 2
   end
 
   it "destroys comments when deleted" do
-    @post = FactoryGirl.create(:post, author: member)
-    @comment1 = FactoryGirl.create(:comment, post: @post)
-    @comment2 = FactoryGirl.create(:comment, post: @post)
-    @post.comments.size.should == 2
+    post = FactoryGirl.create(:post, author: member)
+    FactoryGirl.create(:comment, post: post)
+    FactoryGirl.create(:comment, post: post)
+    post.comments.size.should == 2
     all = Comment.count
-    @post.destroy
+    post.destroy
     Comment.count.should == all - 2
   end
 
   it "belongs to a forum" do
-    @post = FactoryGirl.create(:forum_post)
-    @post.forum.should be_an_instance_of Forum
+    post = FactoryGirl.create(:forum_post)
+    post.forum.should be_an_instance_of Forum
   end
 
   it "doesn't allow a nil subject" do
-    @post = FactoryGirl.build(:post, subject: nil)
-    @post.should_not be_valid
+    post = FactoryGirl.build(:post, subject: nil)
+    post.should_not be_valid
   end
 
   it "doesn't allow a blank subject" do
-    @post = FactoryGirl.build(:post, subject: "")
-    @post.should_not be_valid
+    post = FactoryGirl.build(:post, subject: "")
+    post.should_not be_valid
   end
 
   it "doesn't allow a subject with only spaces" do
-    @post = FactoryGirl.build(:post, subject: "    ")
-    @post.should_not be_valid
+    post = FactoryGirl.build(:post, subject: "    ")
+    post.should_not be_valid
   end
 
   context "recent activity" do
@@ -69,59 +76,68 @@ describe Post do
       Time.stub(now: Time.now)
     end
 
-    let(:post) { FactoryGirl.create(:post, created_at: 1.day.ago) }
+    let!(:post) { FactoryGirl.create(:post, created_at: 1.day.ago) }
 
     it "sets recent activity to post time" do
       post.recent_activity.to_i.should eq post.created_at.to_i
     end
 
     it "sets recent activity to comment time" do
-      @comment = FactoryGirl.create(:comment, post: post,
-                                              created_at: 1.hour.ago)
-      post.recent_activity.to_i.should eq @comment.created_at.to_i
+      comment = FactoryGirl.create(:comment, post: post,
+                                             created_at: 1.hour.ago)
+      post.recent_activity.to_i.should eq comment.created_at.to_i
     end
 
     it "shiny new post is recently active" do
       # create a shiny new post
-      @post2 = FactoryGirl.create(:post, created_at: 1.minute.ago)
-      Post.recently_active.first.should eq @post2
+      post2 = FactoryGirl.create(:post, created_at: 1.minute.ago)
+      Post.recently_active.first.should eq post2
+      Post.recently_active.second.should eq post
     end
 
     it "new comment on old post is recently active" do
       # now comment on an older post
-      @comment2 = FactoryGirl.create(:comment, post: post, created_at: 1.second.ago)
+      post2 = FactoryGirl.create(:post, created_at: 1.minute.ago)
+      FactoryGirl.create(:comment, post: post, created_at: 1.second.ago)
       Post.recently_active.first.should eq post
+      Post.recently_active.second.should eq post2
     end
   end
 
   context "notifications" do
     let(:member2) { FactoryGirl.create(:member) }
 
-    it "sends a notification when a member is mentioned" do
+    it "sends a notification when a member is mentioned using @-syntax" do
       expect {
-        FactoryGirl.create(:post, author: member, body: "Hey @" << member2.login_name)
+        FactoryGirl.create(:post, author: member, body: "Hey @#{member2}")
+      }.to change(Notification, :count).by(1)
+    end
+
+    it "sends a notification when a member is mentioned using [](member) syntax" do
+      expect {
+        FactoryGirl.create(:post, author: member, body: "Hey [#{member2}](member)")
       }.to change(Notification, :count).by(1)
     end
 
     it "sets the notification field" do
-      @p = FactoryGirl.create(:post, author: member, body: "Hey @" << member2.login_name)
-      @n = Notification.first
-      @n.sender.should eq member
-      @n.recipient.should eq member2
-      @n.subject.should match /mentioned you in their post/
-      @n.body.should eq @p.body
+      p = FactoryGirl.create(:post, author: member, body: "Hey @#{member2}")
+      n = Notification.first
+      n.sender.should eq member
+      n.recipient.should eq member2
+      n.subject.should match(/mentioned you in their post/)
+      n.body.should eq p.body
     end
 
     it "sends notifications to all members mentioned" do
-      @member3 = FactoryGirl.create(:member)
+      member3 = FactoryGirl.create(:member)
       expect {
-        FactoryGirl.create(:post, author: member, body: "Hey @" << member2.login_name << " & @" << @member3.login_name)
+        FactoryGirl.create(:post, author: member, body: "Hey @#{member2} & @#{member3}")
       }.to change(Notification, :count).by(2)
     end
 
     it "doesn't send notifications if you mention yourself" do
       expect {
-        FactoryGirl.create(:post, author: member, body: "@" << member.login_name)
+        FactoryGirl.create(:post, author: member, body: "@#{member}")
       }.to change(Notification, :count).by(0)
     end
   end

spec/models/scientific_name_spec.rb

@@ -10,7 +10,7 @@ describe ScientificName do
 
     it 'should be fetchable from the database' do
       sn.save
-      @sn2 = ScientificName.find_by_name('Zea mays')
+      @sn2 = ScientificName.find_by(name: 'Zea mays')
       @sn2.crop.name.should == 'maize'
     end
 

spec/spec_helper.rb

@@ -14,8 +14,8 @@
 # users commonly want.
 #
 # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
-require "codeclimate-test-reporter"
-CodeClimate::TestReporter.start
+require 'simplecov'
+SimpleCov.start
 RSpec.configure do |config|
   # rspec-expectations config goes here. You can use an alternate
   # assertion/expectation library such as wrong or the stdlib/minitest

spec/support/controller_macros.rb

@@ -1,7 +1,7 @@
 # Taken unashamedly from https://github.com/plataformatec/devise/wiki/How-To%3a-Controllers-and-Views-tests-with-Rails-3-%28and-rspec%29
 module ControllerMacros
   def login_member(member_factory = :member)
-    let(:member) { member = FactoryGirl.create(member_factory || :member) }
+    let(:member) { FactoryGirl.create(member_factory || :member) }
     before(:each) do
       @request.env["devise.mapping"] = Devise.mappings[:member]
       sign_in member

spec/support/elasticsearch_helpers.rb

@@ -1,9 +1,8 @@
 module ElasticsearchHelpers
   def sync_elasticsearch(crops)
-    if ENV['GROWSTUFF_ELASTICSEARCH'] == "true"
-      crops.each { |crop| crop.__elasticsearch__.index_document }
-      Crop.__elasticsearch__.refresh_index!
-    end
+    return unless ENV['GROWSTUFF_ELASTICSEARCH'] == "true"
+    crops.each { |crop| crop.__elasticsearch__.index_document }
+    Crop.__elasticsearch__.refresh_index!
   end
 end
 
@@ -11,8 +10,6 @@ RSpec.configure do |config|
   config.include ElasticsearchHelpers
 
   config.before(:all) do
-    if ENV['GROWSTUFF_ELASTICSEARCH'] == "true"
-      Crop.__elasticsearch__.create_index! force: true
-    end
+    Crop.__elasticsearch__.create_index! force: true if ENV['GROWSTUFF_ELASTICSEARCH'] == "true"
   end
 end

spec/views/devise/shared/_links_spec.rb

@@ -6,7 +6,7 @@ describe 'devise/shared/_links.haml', type: "view" do
     dm.stub(confirmable?: confirm)
     dm.stub(lockable?: lock)
     dm.stub(omniauthable?: oauth)
-    return dm
+    dm
   end
 
   it 'should have a sign-in link if not in sessions' do

spec/views/plantings/show.html.haml_spec.rb

@@ -22,7 +22,7 @@ describe "plantings/show" do
     )
   end
 
-  before (:each) do
+  before(:each) do
     @member = FactoryGirl.create(:member)
     controller.stub(:current_user) { @member }
     @p = create_planting_for(@member)