From 93f9435fb95c7d955f2b34c873d97c0ac3130c6a Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 19 Sep 2015 16:24:01 -0400 Subject: [PATCH 01/97] GS-658: sort locale keys, add rake task for it --- config/locales/en.yml | 254 +++++++++++++++++++----------------------- config/locales/ja.yml | 4 +- lib/tasks/i18n.rake | 6 + 3 files changed, 124 insertions(+), 140 deletions(-) create mode 100644 lib/tasks/i18n.rake diff --git a/config/locales/en.yml b/config/locales/en.yml index 8d77afdc8..30b7c2f4f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,153 +1,131 @@ -# Sample localization file for English. Add more files in this directory for other locales. -# See https://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points. - +--- en: - unauthorized: - read: - notification: "You must be signed in to view notifications." - create: - planting: "Please sign in or sign up to plant something." - post: "Please sign in or sign up to post." - seed: "Please sign in or sign up to add seeds." - notification: "Please sign in to send a message." - all: "Please sign in or sign up to create a %{subject}." - manage: - all: "Not authorized to %{action} %{subject}." - - layouts: - header: - skip: "Skip navigation menu" - browse_crops: &browse_crops "Browse Crops" - seeds: "Seeds" - plantings: "Plantings" - harvests: "Harvests" - community_map: "Community Map" - browse_members: "Browse Members" - posts: "Posts" - forums: &forums "Forums" - support_growstuff: "Support Growstuff" - profile: "Profile" - gardens: "Gardens" - account: "Account" - inbox_unread: "Inbox (%{unread_count})" - inbox: "Inbox" - crop_wrangling: "Crop Wrangling" - admin: "Admin" - your_stuff: "Your Stuff (%{unread_count})" - crops: index: - title: *browse_crops subtitle: "%{crops_size} total" - - seeds: + title: Browse Crops + forums: index: - title: - default: "Everyone's seeds" - crop_seeds: "Everyone's %{crop} seeds" - owner_seeds: "%{owner} seeds" - - plantings: - index: - title: - default: "Everyone's plantings" - crop_plantings: "Everyone's %{crop} plantings" - owner_plantings: "%{owner} plantings" - + title: Forums harvests: index: title: - default: "Everyone's harvests" - crop_harvests: "Everyone's %{crop} harvests" + crop_harvests: Everyone's %{crop} harvests + default: Everyone's harvests owner_harvests: "%{owner} harvests" - - places: - index: - title: "%{site_name} Community Map" - - members: - index: - title: "%{site_name} members" - - posts: - index: - title: - default: "Everyone's posts" - author_posts: "%{author} posts" - - shop: - index: - title: "Shop" - - forums: - index: - title: *forums - 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." - perks: "Join now for your free garden journal, seed sharing, forums, and more." - sign_up: "Sign up" - already_html: "Or %{sign_in} if you already have an account" - sign_in_linktext: "sign in" - + perks: Join now for your free garden journal, seed sharing, forums, and more. + sign_in_linktext: sign in + sign_up: Sign up crops: - our_crops: "Some of our crops" - recently_planted: "Recently planted" - recently_added: "Recently added crops" - view_all: "View all crops" - + our_crops: Some of our crops + recently_added: Recently added crops + recently_planted: Recently planted + view_all: View all crops discuss: - discussion: "Discussion" - forums: "Forums" - view_all: "View all posts" - - members: - title: "Some of our members" - view_all: "View all members" - - open: - open_source_title: "Open Source" - 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}." - why_linktext: "why Growstuff is open source" - github_linktext: "Github" - open_data_title: "Open Data and APIs" - 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}." - creative_commons_linktext: "Creative Commons license" - wiki_linktext: "open data" - api_docs_linktext: "API documentation" - get_involved_title: "Get Involved" - 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}." - talk_linktext: "Growstuff Talk" - wiki_linktext: "Growstuff Wiki" - support_title: "Support Growstuff" - support_body_html: "Growstuff is independent, %{ad_free} and we have no outside investment. You can support our work by %{buy_account}." - ad_free_linktext: "ad-free" - buy_account_linktext: "buying a paid account" - - seeds: - title: "Seeds available to trade" - owner: "Owner" - crop: "Crop" - description: "Description" - trade_to: "Will trade to" - from: "From location" - unspecified: "unspecified" - details: "Details" - view_all: "View all seeds" - - stats: - message_html: "So far, %{member} have planted %{number_crops} %{number_plantings} in %{number_gardens}." - member_linktext: "%{count} members" - number_crops_linktext: "%{count} crops" - number_plantings_linktext: "%{count} times" - number_gardens_linktext: "%{count} gardens" - + discussion: Discussion + forums: Forums + view_all: View all posts index: - welcome: "Welcome to %{site_name}, %{member_name}" - plant: "Plant" - harvest: "Harvest" - add_seeds: "Add seeds" - post: "Post" - edit_profile: "Edit profile" - + add_seeds: Add seeds + edit_profile: Edit profile + harvest: Harvest + plant: Plant + post: Post + welcome: Welcome to %{site_name}, %{member_name} + members: + title: Some of our members + view_all: View all members + open: + ad_free_linktext: ad-free + 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_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_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_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_title: Support Growstuff + talk_linktext: Growstuff Talk + why_linktext: why Growstuff is open source + wiki_linktext: Growstuff Wiki + seeds: + crop: Crop + description: Description + details: Details + from: From location + owner: Owner + title: Seeds available to trade + trade_to: Will trade to + unspecified: unspecified + view_all: View all seeds + stats: + member_linktext: "%{count} members" + 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" + layouts: + header: + account: Account + admin: Admin + browse_crops: Browse Crops + browse_members: Browse Members + community_map: Community Map + crop_wrangling: Crop Wrangling + forums: Forums + gardens: Gardens + harvests: Harvests + inbox: Inbox + inbox_unread: Inbox (%{unread_count}) + plantings: Plantings + posts: Posts + profile: Profile + seeds: Seeds + skip: Skip navigation menu + support_growstuff: Support Growstuff + your_stuff: Your Stuff (%{unread_count}) + members: + index: + title: "%{site_name} members" + places: + index: + title: "%{site_name} Community Map" + plantings: + index: + title: + crop_plantings: Everyone's %{crop} plantings + default: Everyone's plantings + owner_plantings: "%{owner} plantings" + posts: + index: + title: + author_posts: "%{author} posts" + default: Everyone's posts + seeds: + index: + title: + crop_seeds: Everyone's %{crop} seeds + default: Everyone's seeds + owner_seeds: "%{owner} seeds" + shop: + index: + title: Shop + unauthorized: + create: + all: Please sign in or sign up to create a %{subject}. + notification: Please sign in to send a message. + planting: Please sign in or sign up to plant something. + post: Please sign in or sign up to post. + seed: Please sign in or sign up to add seeds. + manage: + all: Not authorized to %{action} %{subject}. + read: + notification: You must be signed in to view notifications. diff --git a/config/locales/ja.yml b/config/locales/ja.yml index 925a54399..2a51c32c5 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -1,5 +1,5 @@ +--- ja: home: - blurb: + blurb: intro: "%{site_name}はガーデナーのコミュニティです。" - \ No newline at end of file diff --git a/lib/tasks/i18n.rake b/lib/tasks/i18n.rake new file mode 100644 index 000000000..447f0a5d3 --- /dev/null +++ b/lib/tasks/i18n.rake @@ -0,0 +1,6 @@ +namespace :i18n do + desc "sort all locale keys" + task normalize: :environment do + `i18n-tasks normalize` + end +end From 98581801c3964c89199566abc2bb8d23892cec64 Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 19 Sep 2015 17:45:31 -0400 Subject: [PATCH 02/97] GS-658 - i18n automation POC --- Gemfile | 1 + Gemfile.lock | 11 +++ app/views/layouts/_header.html.haml | 14 ++-- .../layouts/_header.html.i18n-extractor.haml | 78 +++++++++++++++++++ config/locales/en.yml | 32 ++++++-- lib/tasks/i18n.rake | 16 +++- 6 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 app/views/layouts/_header.html.i18n-extractor.haml diff --git a/Gemfile b/Gemfile index 759b1703d..43631a522 100644 --- a/Gemfile +++ b/Gemfile @@ -125,6 +125,7 @@ group :development, :test do gem 'poltergeist', '~> 1.6' # for headless JS testing gem 'i18n-tasks' # adds tests for finding missing and unused translations gem 'selenium-webdriver' + gem 'haml-i18n-extractor' end group :travis do diff --git a/Gemfile.lock b/Gemfile.lock index 54f97ae9b..cc9584247 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -199,6 +199,12 @@ GEM rspec (>= 2.99.0, < 4.0) haml (4.1.0.beta.1) tilt + haml-i18n-extractor (0.5.9) + activesupport + haml + highline + tilt + trollop (= 1.16.2) haml-rails (0.6.0) actionpack (>= 4.0.1) activesupport (>= 4.0.1) @@ -422,6 +428,7 @@ GEM thread_safe (0.3.5) tilt (1.4.1) tins (1.3.3) + trollop (1.16.2) tzinfo (1.2.2) thread_safe (~> 0.1) uglifier (2.5.3) @@ -484,6 +491,7 @@ DEPENDENCIES guard guard-rspec haml + haml-i18n-extractor haml-rails heroku-api i18n-tasks @@ -520,3 +528,6 @@ DEPENDENCIES unicorn webrat will_paginate (~> 3.0) + +BUNDLED WITH + 1.10.6 diff --git a/app/views/layouts/_header.html.haml b/app/views/layouts/_header.html.haml index ddc9c6af2..5523e75fe 100644 --- a/app/views/layouts/_header.html.haml +++ b/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 @@ -20,7 +20,7 @@ %ul.nav.navbar-nav.pull-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 @@ -29,7 +29,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 @@ -44,7 +44,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) @@ -67,11 +67,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 diff --git a/app/views/layouts/_header.html.i18n-extractor.haml b/app/views/layouts/_header.html.i18n-extractor.haml new file mode 100644 index 000000000..5523e75fe --- /dev/null +++ b/app/views/layouts/_header.html.i18n-extractor.haml @@ -0,0 +1,78 @@ +.sr-only + =link_to t(".skip"), "#skipnav" +.navbar.navbar-default.navbar-fixed-top(role="navigation") + .container + .navbar-header + %button.navbar-toggle(data-target="#navbar-collapse" data-toggle="collapse") + %span.sr-only= t('.toggle_navigation') + %span.icon-bar + %span.icon-bar + %span.icon-bar + %a.navbar-brand(href=root_path) + = image_tag("growstuff-brand.png", :size => "200x50", :alt => ENV['GROWSTUFF_SITE_NAME']) + = form_tag crops_search_path, :method => :get, :id => 'navbar-search', :class => 'navbar-form pull-right' do + .input + = label_tag :term, "Search crop database:", :class => 'sr-only' + = text_field_tag 'term', nil, :class => 'search-query input-medium form-control', :placeholder => 'Search crops' + = submit_tag "Search", :class => 'btn sr-only' + + .navbar-collapse.collapse#navbar-collapse + %ul.nav.navbar-nav.pull-right + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => crops_path} + = t('.crops') + %b.caret + %ul.dropdown-menu + %li= link_to t('.browse_crops'), crops_path + %li= link_to t('.seeds'), seeds_path + %li= link_to t('.plantings'), plantings_path + %li= link_to t('.harvests'), harvests_path + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => members_path} + = t('.community') + %b.caret + %ul.dropdown-menu + %li= link_to t('.community_map'), places_path + %li= link_to t('.browse_members'), members_path + %li= link_to t('.posts'), posts_path + %li= link_to t('.forums'), forums_path + %li= link_to t('.support_growstuff'), shop_path + + - if member_signed_in? + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => root_path} + - if current_member.notifications.unread_count > 0 + = t('.your_stuff', unread_count: current_member.notifications.unread_count) + - else + = 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) + %li= link_to t('.gardens'), gardens_by_owner_path(:owner => current_member.slug) + %li= link_to t('.plantings'), plantings_by_owner_path(:owner => current_member.slug) + %li= link_to t('.harvest'), harvests_by_owner_path(:owner => current_member.slug) + %li= link_to t('.seeds'), seeds_by_owner_path(:owner => current_member.slug) + %li= link_to t('.posts'), posts_by_author_path(:author => current_member.slug) + %li= link_to t('.account'), orders_path + %li + - if current_member.notifications.unread_count > 0 + = link_to(t('.inbox_unread', unread_count: current_member.notifications.unread_count), notifications_path) + - else + = link_to(t('.inbox'), notifications_path) + - if current_member.has_role?(:crop_wrangler) || current_member.has_role?(:admin) + %li{:class => 'divider', :role => 'presentation'} + - if current_member.has_role?(:crop_wrangler) + %li= link_to t('.crop_wrangling'), wrangle_crops_path + - if current_member.has_role?(:admin) + %li= link_to t('.admin'), admin_path + + + %li= link_to t('.sign_out'), destroy_member_session_path, :method => :delete + + - else + %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 +%a{:name => 'skipnav'} diff --git a/config/locales/en.yml b/config/locales/en.yml index 30b7c2f4f..6d9b47772 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -16,7 +16,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 @@ -44,14 +46,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 @@ -68,7 +80,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" @@ -92,6 +105,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" diff --git a/lib/tasks/i18n.rake b/lib/tasks/i18n.rake index 447f0a5d3..917cd73f6 100644 --- a/lib/tasks/i18n.rake +++ b/lib/tasks/i18n.rake @@ -1,6 +1,18 @@ namespace :i18n do - desc "sort all locale keys" - task normalize: :environment 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 From e02a6e569c0f996ef0859e567fa11102ec0d7850 Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 26 Sep 2015 11:54:02 -0400 Subject: [PATCH 03/97] Remove example file, add documentation --- .../layouts/_header.html.i18n-extractor.haml | 78 ------------------- config/locales/README.md | 15 ++++ 2 files changed, 15 insertions(+), 78 deletions(-) delete mode 100644 app/views/layouts/_header.html.i18n-extractor.haml create mode 100644 config/locales/README.md diff --git a/app/views/layouts/_header.html.i18n-extractor.haml b/app/views/layouts/_header.html.i18n-extractor.haml deleted file mode 100644 index 5523e75fe..000000000 --- a/app/views/layouts/_header.html.i18n-extractor.haml +++ /dev/null @@ -1,78 +0,0 @@ -.sr-only - =link_to t(".skip"), "#skipnav" -.navbar.navbar-default.navbar-fixed-top(role="navigation") - .container - .navbar-header - %button.navbar-toggle(data-target="#navbar-collapse" data-toggle="collapse") - %span.sr-only= t('.toggle_navigation') - %span.icon-bar - %span.icon-bar - %span.icon-bar - %a.navbar-brand(href=root_path) - = image_tag("growstuff-brand.png", :size => "200x50", :alt => ENV['GROWSTUFF_SITE_NAME']) - = form_tag crops_search_path, :method => :get, :id => 'navbar-search', :class => 'navbar-form pull-right' do - .input - = label_tag :term, "Search crop database:", :class => 'sr-only' - = text_field_tag 'term', nil, :class => 'search-query input-medium form-control', :placeholder => 'Search crops' - = submit_tag "Search", :class => 'btn sr-only' - - .navbar-collapse.collapse#navbar-collapse - %ul.nav.navbar-nav.pull-right - %li.dropdown< - %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => crops_path} - = t('.crops') - %b.caret - %ul.dropdown-menu - %li= link_to t('.browse_crops'), crops_path - %li= link_to t('.seeds'), seeds_path - %li= link_to t('.plantings'), plantings_path - %li= link_to t('.harvests'), harvests_path - %li.dropdown< - %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => members_path} - = t('.community') - %b.caret - %ul.dropdown-menu - %li= link_to t('.community_map'), places_path - %li= link_to t('.browse_members'), members_path - %li= link_to t('.posts'), posts_path - %li= link_to t('.forums'), forums_path - %li= link_to t('.support_growstuff'), shop_path - - - if member_signed_in? - %li.dropdown< - %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => root_path} - - if current_member.notifications.unread_count > 0 - = t('.your_stuff', unread_count: current_member.notifications.unread_count) - - else - = 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) - %li= link_to t('.gardens'), gardens_by_owner_path(:owner => current_member.slug) - %li= link_to t('.plantings'), plantings_by_owner_path(:owner => current_member.slug) - %li= link_to t('.harvest'), harvests_by_owner_path(:owner => current_member.slug) - %li= link_to t('.seeds'), seeds_by_owner_path(:owner => current_member.slug) - %li= link_to t('.posts'), posts_by_author_path(:author => current_member.slug) - %li= link_to t('.account'), orders_path - %li - - if current_member.notifications.unread_count > 0 - = link_to(t('.inbox_unread', unread_count: current_member.notifications.unread_count), notifications_path) - - else - = link_to(t('.inbox'), notifications_path) - - if current_member.has_role?(:crop_wrangler) || current_member.has_role?(:admin) - %li{:class => 'divider', :role => 'presentation'} - - if current_member.has_role?(:crop_wrangler) - %li= link_to t('.crop_wrangling'), wrangle_crops_path - - if current_member.has_role?(:admin) - %li= link_to t('.admin'), admin_path - - - %li= link_to t('.sign_out'), destroy_member_session_path, :method => :delete - - - else - %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 -%a{:name => 'skipnav'} diff --git a/config/locales/README.md b/config/locales/README.md new file mode 100644 index 000000000..5ce22aa89 --- /dev/null +++ b/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 From 2f561aaa473120cceb88be3be21fcc657cc7729e Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 26 Sep 2015 11:54:51 -0400 Subject: [PATCH 04/97] Add myself to contributors --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index deab23739..151d061a4 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -66,3 +66,4 @@ submit the change with your pull request. - Anthony Atkinson / [sha1sum](https://github.com/sha1sum) - Terence Conquest / [twconquest](https://github.com/twconquest) - Daniel O'Connor / [CloCkWeRX](https://github.com/CloCkWeRX) +- Jim Stallings / [jestallin](https://github.com/jestallin) From 494790bcd463bb91f35191699774c6b7e8cdb6d8 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Tue, 11 Oct 2016 15:28:08 -0400 Subject: [PATCH 05/97] moving where clauses into scopes to keep that contained in models --- app/controllers/notifications_controller.rb | 2 +- app/controllers/orders_controller.rb | 2 +- app/models/notification.rb | 4 ++++ app/models/order.rb | 4 ++++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 57b5836ba..cf50316a6 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -5,7 +5,7 @@ class NotificationsController < ApplicationController # GET /notifications def index - @notifications = Notification.where(recipient_id: current_member).page(params[:page]) + @notifications = Notification.find_by_recipient(current_member).page(params[:page]) respond_to do |format| format.html # index.html.erb diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb index 82bb337f9..5de8e46e4 100644 --- a/app/controllers/orders_controller.rb +++ b/app/controllers/orders_controller.rb @@ -4,7 +4,7 @@ class OrdersController < ApplicationController # GET /orders def index - @orders = Order.where(member_id: current_member.id) + @orders = Order.by_member_id(current_member.id) respond_to do |format| format.html # index.html.erb diff --git a/app/models/notification.rb b/app/models/notification.rb index 567867b00..9f9a61257 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -11,6 +11,10 @@ class Notification < ActiveRecord::Base before_create :replace_blank_subject after_create :send_email + def self.find_by_recipient(recipient) + where(recipient_id: recipient) + end + def self.unread_count self.unread.size end diff --git a/app/models/order.rb b/app/models/order.rb index f3a4eaafb..acf3a5878 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -12,6 +12,10 @@ class Order < ActiveRecord::Base before_save :standardize_referral_code + def by_member_id(member_id) + where(member_id: member_id) + end + # total price of an order def total sum = 0 From cee3d192e092e1807d8c0e27e773a8f1e26de437 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Tue, 11 Oct 2016 16:34:25 -0400 Subject: [PATCH 06/97] test the new order.by_member_id scope --- app/models/order.rb | 2 +- spec/models/order_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/models/order.rb b/app/models/order.rb index acf3a5878..00b0b712d 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -12,7 +12,7 @@ class Order < ActiveRecord::Base before_save :standardize_referral_code - def by_member_id(member_id) + def self.by_member_id(member_id) where(member_id: member_id) end diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 9291e1a3f..6a74baf11 100644 --- a/spec/models/order_spec.rb +++ b/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_id(@member1.id).should eq [@order1] + end + end + it 'has order_items' do @order.order_items.first.should eq @order_item end From bcdd2a9167390c23507556d195d8c4d979a19dec Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 15:55:38 +1300 Subject: [PATCH 07/97] Changed called to rails dynamic find_by --- .rubocop_todo.yml | 24 ------------------- app/controllers/alternate_names_controller.rb | 2 +- app/controllers/comments_controller.rb | 2 +- app/controllers/gardens_controller.rb | 2 +- app/controllers/harvests_controller.rb | 6 ++--- app/controllers/notifications_controller.rb | 4 ++-- app/controllers/plantings_controller.rb | 8 +++---- app/controllers/posts_controller.rb | 4 ++-- .../scientific_names_controller.rb | 2 +- app/controllers/seeds_controller.rb | 6 ++--- app/models/crop.rb | 10 ++++---- app/models/member.rb | 2 +- app/models/order.rb | 8 +++---- db/seeds.rb | 2 +- lib/tasks/growstuff.rake | 12 +++++----- spec/controllers/roles_controller_spec.rb | 2 +- spec/models/crop_spec.rb | 2 +- spec/models/scientific_name_spec.rb | 2 +- 18 files changed, 38 insertions(+), 62 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 72007840e..ecf35d1e1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -162,30 +162,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. diff --git a/app/controllers/alternate_names_controller.rb b/app/controllers/alternate_names_controller.rb index bfcbb2a4f..a02c0d9cb 100644 --- a/app/controllers/alternate_names_controller.rb +++ b/app/controllers/alternate_names_controller.rb @@ -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_by(id: params[:crop_id]) || Crop.new respond_to do |format| format.html # new.html.haml diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 0f078e2dd..967a386f8 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -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 diff --git a/app/controllers/gardens_controller.rb b/app/controllers/gardens_controller.rb index 5820b83d3..8720a693d 100644 --- a/app/controllers/gardens_controller.rb +++ b/app/controllers/gardens_controller.rb @@ -6,7 +6,7 @@ class GardensController < ApplicationController # GET /gardens.json def index @gardens = Garden.paginate(page: params[:page]) - @owner = Member.find_by_slug(params[:owner]) + @owner = Member.find_by(slug: params[:owner]) if @owner @gardens = @owner.gardens.paginate(page: params[:page]) end diff --git a/app/controllers/harvests_controller.rb b/app/controllers/harvests_controller.rb index 051cb1132..948edfb63 100644 --- a/app/controllers/harvests_controller.rb +++ b/app/controllers/harvests_controller.rb @@ -5,8 +5,8 @@ class HarvestsController < ApplicationController # 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_by(id: params[:crop_id]) || Crop.new respond_to do |format| format.html # new.html.erb diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index b6e0acbec..39f48b85d 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -28,7 +28,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| @@ -66,7 +66,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 diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb index 2c4589e40..222944151 100644 --- a/app/controllers/plantings_controller.rb +++ b/app/controllers/plantings_controller.rb @@ -5,8 +5,8 @@ class PlantingsController < ApplicationController # 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 diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 48216ac87..75a4a7d49 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -6,7 +6,7 @@ class PostsController < ApplicationController # 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 diff --git a/app/controllers/scientific_names_controller.rb b/app/controllers/scientific_names_controller.rb index cc7723b0b..1bbfcd4c6 100644 --- a/app/controllers/scientific_names_controller.rb +++ b/app/controllers/scientific_names_controller.rb @@ -28,7 +28,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_by(id: params[:crop_id]) || Crop.new respond_to do |format| format.html # new.html.haml diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb index 188855f45..45b14cb16 100644 --- a/app/controllers/seeds_controller.rb +++ b/app/controllers/seeds_controller.rb @@ -5,8 +5,8 @@ class SeedsController < ApplicationController # 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 @@ -49,7 +49,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_by(id: params[:crop_id]) || Crop.new respond_to do |format| format.html # new.html.erb diff --git a/app/models/crop.rb b/app/models/crop.rb index 47829f463..7bc50c400 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -241,7 +241,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) @@ -251,7 +251,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 @@ -274,7 +274,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength end if names_to_add.size > 0 - 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 names_to_add.each do |n| @@ -294,7 +294,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength def add_alternate_names_from_csv(alternate_names) names_to_add = [] if !alternate_names.blank? # i.e. we actually passed something in, which isn't a given - cropbot = Member.find_by_login_name('cropbot') + cropbot = Member.find_by(login_name: 'cropbot') raise "cropbot account not found: run rake db:seed" unless cropbot names_to_add = alternate_names.split(%r{,\s*}) @@ -354,7 +354,7 @@ 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) diff --git a/app/models/member.rb b/app/models/member.rb index 018860365..331328988 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -137,7 +137,7 @@ class Member < ActiveRecord::Base end def auth(provider) - return authentications.find_by_provider(provider) + return authentications.find_by(provider: provider) end # Authenticates against Flickr and returns an object we can use for subsequent api calls diff --git a/app/models/order.rb b/app/models/order.rb index df8609184..c5d19b7cd 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -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 diff --git a/db/seeds.rb b/db/seeds.rb index 6503cde74..cf38dd1d0 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -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 diff --git a/lib/tasks/growstuff.rake b/lib/tasks/growstuff.rake index fd0258309..cbce26684 100644 --- a/lib/tasks/growstuff.rake +++ b/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,13 @@ 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 = Crop.find_by(name: crop_name) if crop alternate_names.split(/,\s*/).each do |an| AlternateName.where( diff --git a/spec/controllers/roles_controller_spec.rb b/spec/controllers/roles_controller_spec.rb index f62289379..5c2271bce 100644 --- a/spec/controllers/roles_controller_spec.rb +++ b/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 diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index b497b6222..e60d56290 100644 --- a/spec/models/crop_spec.rb +++ b/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 diff --git a/spec/models/scientific_name_spec.rb b/spec/models/scientific_name_spec.rb index b4b498db0..673655b92 100644 --- a/spec/models/scientific_name_spec.rb +++ b/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_scientific_name('Zea mays') + @sn2 = ScientificName.find_by(scientific_name: 'Zea mays') @sn2.crop.name.should == 'maize' end From 07c135eeb5a71804a6c6cae080283f9a6583f119 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 17:20:13 +1300 Subject: [PATCH 08/97] before_filter is now before_action --- .rubocop_todo.yml | 27 ------------------- app/controllers/account_types_controller.rb | 2 +- app/controllers/accounts_controller.rb | 2 +- app/controllers/alternate_names_controller.rb | 2 +- app/controllers/application_controller.rb | 4 +-- app/controllers/authentications_controller.rb | 2 +- app/controllers/comments_controller.rb | 2 +- app/controllers/crops_controller.rb | 2 +- app/controllers/follows_controller.rb | 2 +- app/controllers/gardens_controller.rb | 2 +- app/controllers/harvests_controller.rb | 2 +- app/controllers/notifications_controller.rb | 2 +- app/controllers/order_items_controller.rb | 2 +- app/controllers/orders_controller.rb | 2 +- app/controllers/plantings_controller.rb | 2 +- app/controllers/posts_controller.rb | 2 +- app/controllers/products_controller.rb | 2 +- app/controllers/roles_controller.rb | 2 +- .../scientific_names_controller.rb | 2 +- app/controllers/seeds_controller.rb | 2 +- 20 files changed, 20 insertions(+), 47 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index ecf35d1e1..8892c96d3 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -121,33 +121,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 diff --git a/app/controllers/account_types_controller.rb b/app/controllers/account_types_controller.rb index 073933af4..fce71380a 100644 --- a/app/controllers/account_types_controller.rb +++ b/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 diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index b2daf95ed..bca64597a 100644 --- a/app/controllers/accounts_controller.rb +++ b/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 diff --git a/app/controllers/alternate_names_controller.rb b/app/controllers/alternate_names_controller.rb index a02c0d9cb..ef81497e1 100644 --- a/app/controllers/alternate_names_controller.rb +++ b/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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 45c657b64..8548f8e8e 100644 --- a/app/controllers/application_controller.rb +++ b/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" && diff --git a/app/controllers/authentications_controller.rb b/app/controllers/authentications_controller.rb index 886715373..3567ea67f 100644 --- a/app/controllers/authentications_controller.rb +++ b/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 diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 967a386f8..ef3225bfb 100644 --- a/app/controllers/comments_controller.rb +++ b/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 diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb index dac763665..3c665e9d1 100644 --- a/app/controllers/crops_controller.rb +++ b/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] diff --git a/app/controllers/follows_controller.rb b/app/controllers/follows_controller.rb index 58a1ce795..2c4b2daab 100644 --- a/app/controllers/follows_controller.rb +++ b/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 diff --git a/app/controllers/gardens_controller.rb b/app/controllers/gardens_controller.rb index 8720a693d..a83089844 100644 --- a/app/controllers/gardens_controller.rb +++ b/app/controllers/gardens_controller.rb @@ -1,5 +1,5 @@ class GardensController < ApplicationController - before_filter :authenticate_member!, except: [:index, :show] + before_action :authenticate_member!, except: [:index, :show] load_and_authorize_resource # GET /gardens diff --git a/app/controllers/harvests_controller.rb b/app/controllers/harvests_controller.rb index 948edfb63..3d36438fd 100644 --- a/app/controllers/harvests_controller.rb +++ b/app/controllers/harvests_controller.rb @@ -1,5 +1,5 @@ class HarvestsController < ApplicationController - before_filter :authenticate_member!, except: [:index, :show] + before_action :authenticate_member!, except: [:index, :show] load_and_authorize_resource # GET /harvests diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 39f48b85d..4bdae1c90 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,6 +1,6 @@ class NotificationsController < ApplicationController include NotificationsHelper - before_filter :authenticate_member! + before_action :authenticate_member! load_and_authorize_resource # GET /notifications diff --git a/app/controllers/order_items_controller.rb b/app/controllers/order_items_controller.rb index 7781913d4..0eb381a89 100644 --- a/app/controllers/order_items_controller.rb +++ b/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 diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb index 234c760f2..a3def3194 100644 --- a/app/controllers/orders_controller.rb +++ b/app/controllers/orders_controller.rb @@ -1,5 +1,5 @@ class OrdersController < ApplicationController - before_filter :authenticate_member! + before_action :authenticate_member! load_and_authorize_resource # GET /orders diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb index 222944151..708fc448d 100644 --- a/app/controllers/plantings_controller.rb +++ b/app/controllers/plantings_controller.rb @@ -1,5 +1,5 @@ class PlantingsController < ApplicationController - before_filter :authenticate_member!, except: [:index, :show] + before_action :authenticate_member!, except: [:index, :show] load_and_authorize_resource # GET /plantings diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 75a4a7d49..76befd29d 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -1,5 +1,5 @@ class PostsController < ApplicationController - before_filter :authenticate_member!, except: [:index, :show] + before_action :authenticate_member!, except: [:index, :show] load_and_authorize_resource # GET /posts diff --git a/app/controllers/products_controller.rb b/app/controllers/products_controller.rb index be3e923dd..cb2f96e52 100644 --- a/app/controllers/products_controller.rb +++ b/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 diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 82b554467..91bcf2e52 100644 --- a/app/controllers/roles_controller.rb +++ b/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 diff --git a/app/controllers/scientific_names_controller.rb b/app/controllers/scientific_names_controller.rb index 1bbfcd4c6..20c8dae0b 100644 --- a/app/controllers/scientific_names_controller.rb +++ b/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 diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb index 45b14cb16..5d9a775e7 100644 --- a/app/controllers/seeds_controller.rb +++ b/app/controllers/seeds_controller.rb @@ -1,5 +1,5 @@ class SeedsController < ApplicationController - before_filter :authenticate_member!, except: [:index, :show] + before_action :authenticate_member!, except: [:index, :show] load_and_authorize_resource # GET /seeds From 8196d94a73e9a8ccc886ef92cf9874111f8a6e19 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 17:23:31 +1300 Subject: [PATCH 09/97] Don't enforce http pos args until rails 5 --- .rubocop_todo.yml | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8892c96d3..c650b322b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -159,28 +159,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 From b541fe8221d5823a5db2ea7aa5fa74cde368aa5f Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 17:24:41 +1300 Subject: [PATCH 10/97] puts allowed in some files --- .rubocop.yml | 7 +++++++ .rubocop_todo.yml | 8 -------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 2631b0f0d..d2bb99356 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -66,3 +66,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' \ No newline at end of file diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c650b322b..6bf2490c9 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -159,14 +159,6 @@ Rails/HasAndBelongsToMany: - 'app/models/product.rb' - 'app/models/role.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: From d60dcfbddd21dded43cf891863888dfc69bb1e43 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 17:26:04 +1300 Subject: [PATCH 11/97] Use request.referer instead of request.referrer --- .rubocop_todo.yml | 8 -------- app/controllers/application_controller.rb | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6bf2490c9..a5fcfe3b4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -173,14 +173,6 @@ Rails/PluralizationGrammar: - '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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8548f8e8e..468f767c6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -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 From 1fd723301256002678de401fdd63e1872192809f Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 17:27:13 +1300 Subject: [PATCH 12/97] Plualisation fixups --- .rubocop_todo.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a5fcfe3b4..487f78c23 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -166,13 +166,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: 9 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: strict, flexible From 8c7fcd9d49119ceab1b62c3e5e53204c14d20326 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 26 Nov 2016 19:27:04 +1300 Subject: [PATCH 13/97] Fixed all useless assignments --- .rubocop_todo.yml | 204 ------------------ app/controllers/crops_controller.rb | 4 +- app/models/crop.rb | 2 - app/models/member.rb | 21 +- config.rb | 2 +- config/compass.rb | 1 + config/setup_load_paths.rb | 2 - db/seeds.rb | 2 +- lib/tasks/growstuff.rake | 6 +- .../admin/orders_controller_spec.rb | 1 - .../plantings/planting_a_crop_spec.rb | 2 +- spec/features/signin_spec.rb | 2 +- spec/features/unsubscribing_spec.rb | 2 +- spec/helpers/gardens_helper_spec.rb | 11 +- spec/helpers/notifications_helper_spec.rb | 2 - spec/helpers/seeds_helper_spec.rb | 1 - spec/models/crop_spec.rb | 44 ++-- spec/models/garden_spec.rb | 2 +- spec/models/member_spec.rb | 15 +- spec/support/controller_macros.rb | 2 +- 20 files changed, 54 insertions(+), 274 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 487f78c23..c39277dd4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -82,28 +82,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: @@ -319,188 +297,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: diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb index 3c665e9d1..7589b8734 100644 --- a/app/controllers/crops_controller.rb +++ b/app/controllers/crops_controller.rb @@ -172,14 +172,14 @@ class CropsController < ApplicationController end params[:alt_name].each do |index, value| - alt_name = @crop.alternate_names.create(name: value, creator_id: current_member.id) + @crop.alternate_names.create(name: value, creator_id: current_member.id) end @crop.scientific_names.each do |sci_name| sci_name.destroy end params[:sci_name].each do |index, value| - sci_name = @crop.scientific_names.create(scientific_name: value, creator_id: current_member.id) + @crop.scientific_names.create(scientific_name: value, creator_id: current_member.id) end end diff --git a/app/models/crop.rb b/app/models/crop.rb index 7bc50c400..614b25b03 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -292,7 +292,6 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength end def add_alternate_names_from_csv(alternate_names) - names_to_add = [] if !alternate_names.blank? # i.e. we actually passed something in, which isn't a given cropbot = Member.find_by(login_name: 'cropbot') raise "cropbot account not found: run rake db:seed" unless cropbot @@ -309,7 +308,6 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength ) end end - end end diff --git a/app/models/member.rb b/app/models/member.rb index 331328988..ea14ccd9b 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -159,7 +159,6 @@ class Member < ActiveRecord::Base # 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, @@ -237,21 +236,21 @@ class Member < ActiveRecord::Base def newsletter_subscribe(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) 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) diff --git a/config.rb b/config.rb index bb4bb0a7d..f816551c1 100644 --- a/config.rb +++ b/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" diff --git a/config/compass.rb b/config/compass.rb index 2b22d5d7c..306cdd02c 100644 --- a/config/compass.rb +++ b/config/compass.rb @@ -1,2 +1,3 @@ # Require any additional compass plugins here. +# rubocop:disable Lint/UselessAssignment project_type = :rails diff --git a/config/setup_load_paths.rb b/config/setup_load_paths.rb index 7c6e7e133..c1a0f95fd 100644 --- a/config/setup_load_paths.rb +++ b/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 diff --git a/db/seeds.rb b/db/seeds.rb index cf38dd1d0..467fd39f2 100644 --- a/db/seeds.rb +++ b/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) diff --git a/lib/tasks/growstuff.rake b/lib/tasks/growstuff.rake index cbce26684..de4c34bc5 100644 --- a/lib/tasks/growstuff.rake +++ b/lib/tasks/growstuff.rake @@ -296,10 +296,8 @@ namespace :growstuff do puts "Loading alternate names from #{file}..." 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_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| diff --git a/spec/controllers/admin/orders_controller_spec.rb b/spec/controllers/admin/orders_controller_spec.rb index 4c407c34d..07fe22170 100644 --- a/spec/controllers/admin/orders_controller_spec.rb +++ b/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 diff --git a/spec/features/plantings/planting_a_crop_spec.rb b/spec/features/plantings/planting_a_crop_spec.rb index 2494e7e41..affd691cb 100644 --- a/spec/features/plantings/planting_a_crop_spec.rb +++ b/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 diff --git a/spec/features/signin_spec.rb b/spec/features/signin_spec.rb index 41aa1ecd1..7bc08147c 100644 --- a/spec/features/signin_spec.rb +++ b/spec/features/signin_spec.rb @@ -66,7 +66,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 diff --git a/spec/features/unsubscribing_spec.rb b/spec/features/unsubscribing_spec.rb index ba231d1d8..d5da11160 100644 --- a/spec/features/unsubscribing_spec.rb +++ b/spec/features/unsubscribing_spec.rb @@ -52,7 +52,7 @@ 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 + Member.find(member.id) # reload the member expect(member.send_planting_reminder).to eq(true) expect(member.send_notification_email).to eq(true) end diff --git a/spec/helpers/gardens_helper_spec.rb b/spec/helpers/gardens_helper_spec.rb index c561c32d1..82a729774 100644 --- a/spec/helpers/gardens_helper_spec.rb +++ b/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 = "
  • " @@ -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) diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb index d79ea409d..152791034 100644 --- a/spec/helpers/notifications_helper_spec.rb +++ b/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) diff --git a/spec/helpers/seeds_helper_spec.rb b/spec/helpers/seeds_helper_spec.rb index c86dc2637..a0eeb49f5 100644 --- a/spec/helpers/seeds_helper_spec.rb +++ b/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 diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index e60d56290..a7bf1037c 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -157,25 +157,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 @@ -184,25 +184,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 diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb index b56ac2056..cd0b25b35 100644 --- a/spec/models/garden_spec.rb +++ b/spec/models/garden_spec.rb @@ -75,7 +75,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 diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index cc6281fc4..19e0a9568 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -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 @@ -273,7 +274,7 @@ describe 'member' do @member1.updated_at = 3.days.ago @member2.updated_at = 2.days.ago - @member3.updated_at = 1.days.ago + @member3.updated_at = 1.day.ago Member.interesting.should eq [@member3, @member2, @member1] end diff --git a/spec/support/controller_macros.rb b/spec/support/controller_macros.rb index 393d7221b..1bf679afb 100644 --- a/spec/support/controller_macros.rb +++ b/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 From 9dc02cd3d8e6fd250a87661a7c426f4f0e0cc42c Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Tue, 29 Nov 2016 12:10:48 -0500 Subject: [PATCH 14/97] make changes from code review --- app/controllers/crops_controller.rb | 11 ++++++----- app/controllers/notifications_controller.rb | 2 +- app/controllers/orders_controller.rb | 2 +- app/models/notification.rb | 4 +--- app/models/order.rb | 4 +--- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb index dac763665..c77522b8f 100644 --- a/app/controllers/crops_controller.rb +++ b/app/controllers/crops_controller.rb @@ -10,14 +10,11 @@ class CropsController < ApplicationController 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]) + @crops = popular_crops end + @paginated_crops = @crops.approved.paginate(page: params[:page]) respond_to do |format| format.html @@ -212,6 +209,10 @@ class CropsController < ApplicationController private + def popular_crops + Crop.popular.includes(:scientific_names, { plantings: :photos }) + end + def crop_params params.require(:crop).permit(:en_wikipedia_url, :name, diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 0f8d38d19..5794a45cc 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -5,7 +5,7 @@ class NotificationsController < ApplicationController # GET /notifications def index - @notifications = Notification.find_by_recipient(current_member).page(params[:page]) + @notifications = Notification.by_recipient(current_member).page(params[:page]) respond_to do |format| format.html # index.html.erb diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb index b0d744268..3e0e97806 100644 --- a/app/controllers/orders_controller.rb +++ b/app/controllers/orders_controller.rb @@ -4,7 +4,7 @@ class OrdersController < ApplicationController # GET /orders def index - @orders = Order.by_member_id(current_member.id) + @orders = Order.by_member(current_member) respond_to do |format| format.html # index.html.erb diff --git a/app/models/notification.rb b/app/models/notification.rb index 4be21e668..329ff17e5 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -7,13 +7,11 @@ 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 - def self.find_by_recipient(recipient) - where(recipient_id: recipient) - end def self.unread_count self.unread.size diff --git a/app/models/order.rb b/app/models/order.rb index df074c5a9..85a3c526d 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -12,9 +12,7 @@ class Order < ActiveRecord::Base before_save :standardize_referral_code - def self.by_member_id(member_id) - where(member_id: member_id) - end + scope :by_member, ->(member) { where(member: member) } # total price of an order def total From 99c8db72d6fee271e41213b632f4d9e6dff769ff Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Tue, 29 Nov 2016 12:14:16 -0500 Subject: [PATCH 15/97] syntax --- app/controllers/crops_controller.rb | 2 ++ app/models/notification.rb | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb index 27fd6a23b..d3a7c1b9b 100644 --- a/app/controllers/crops_controller.rb +++ b/app/controllers/crops_controller.rb @@ -197,6 +197,8 @@ class CropsController < ApplicationController 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) diff --git a/app/models/notification.rb b/app/models/notification.rb index 329ff17e5..f19de9b34 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -7,12 +7,11 @@ class Notification < ActiveRecord::Base default_scope { order('created_at DESC') } scope :unread, -> { where(read: false) } - scope :by_recipient, ->(recipient) {where(recipient_id: recipient)} + scope :by_recipient, ->(recipient) { where(recipient_id: recipient) } before_create :replace_blank_subject after_create :send_email - def self.unread_count self.unread.size end From 4857fd8d2e86b80df59c957eafb631ec78f2032f Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Tue, 29 Nov 2016 12:20:17 -0500 Subject: [PATCH 16/97] return from conditional --- app/controllers/crops_controller.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb index d3a7c1b9b..f357e24fb 100644 --- a/app/controllers/crops_controller.rb +++ b/app/controllers/crops_controller.rb @@ -9,11 +9,11 @@ class CropsController < ApplicationController # GET /crops.json def index @sort = params[:sort] - if @sort == 'alpha' - @crops = Crop.includes(:scientific_names, { plantings: :photos }) - else - @crops = popular_crops - 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| From 72877aebaf30adfddc801441dac457f5117c3ccf Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Tue, 29 Nov 2016 13:41:05 -0500 Subject: [PATCH 17/97] update test for renamed scope --- spec/models/order_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 30e6cf261..1f8bf88bf 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -17,7 +17,7 @@ describe Order do end it "only returns orders belonging to member" do - Order.by_member_id(@member1.id).should eq [@order1] + Order.by_member(@member1).should eq [@order1] end end From b6136d6a201c10dc863b43e12edc6bf191d9cac8 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 30 Nov 2016 10:38:00 +1300 Subject: [PATCH 18/97] Removed ignores for rails findby --- .rubocop_todo.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index dd53f3874..9d0b083c2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -107,15 +107,6 @@ Rails/Date: - 'spec/features/plantings/planting_a_crop_spec.rb' - 'spec/features/shared_examples/append_date.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 From a2e86b3e5e5e8e1ef1f54903cc1341411d85f076 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 30 Nov 2016 12:21:27 +1300 Subject: [PATCH 19/97] Rails dynamic finds fixed --- app/models/crop.rb | 6 +++--- spec/models/scientific_name_spec.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index c40ad47d9..db44bbbff 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -259,7 +259,7 @@ 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 @@ -269,7 +269,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength end def add_alternate_names_from_csv(alternate_names) - cropbot = Member.find_by_login_name('cropbot') + cropbot = Member.find_by(login_name: 'cropbot') 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 @@ -343,7 +343,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 diff --git a/spec/models/scientific_name_spec.rb b/spec/models/scientific_name_spec.rb index 3be2f36ac..b7648cb36 100644 --- a/spec/models/scientific_name_spec.rb +++ b/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 From 77b7969fc98d39b0f930f29489e00c0897dbcadc Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Wed, 30 Nov 2016 00:46:58 -0500 Subject: [PATCH 20/97] DRY up photo & model interaction * move stuff in models into a concern * create combined collection for all models that have photos --- app/models/concerns/photo_capable.rb | 18 ++++++++++++++++++ app/models/garden.rb | 14 ++------------ app/models/harvest.rb | 13 ++----------- app/models/photo.rb | 24 ++++++++++++++---------- app/models/planting.rb | 12 +----------- spec/models/photo_spec.rb | 2 ++ 6 files changed, 39 insertions(+), 44 deletions(-) create mode 100644 app/models/concerns/photo_capable.rb diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb new file mode 100644 index 000000000..f2f53aa42 --- /dev/null +++ b/app/models/concerns/photo_capable.rb @@ -0,0 +1,18 @@ +module PhotoCapable + extend ActiveSupport::Concern + + included do + has_and_belongs_to_many :photos + + before_destroy :remove_from_list + end + + def remove_from_list + photolist = self.photos.to_a # save a temp copy of the photo list + self.photos.clear # clear relationship b/w object and photo + + photolist.each do |photo| + photo.destroy_if_unused + end + end +end diff --git a/app/models/garden.rb b/app/models/garden.rb index ec4aea94f..9f58ac0ab 100644 --- a/app/models/garden.rb +++ b/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 diff --git a/app/models/harvest.rb b/app/models/harvest.rb index 04f4608f2..1afdf473d 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -1,22 +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') } diff --git a/app/models/photo.rb b/app/models/photo.rb index 6d7fdbcba..bd41b5c39 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -1,22 +1,26 @@ class Photo < ActiveRecord::Base + ON_MODELS = %w(plantings harvests gardens) 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 + ON_MODELS.each do |relation| + has_and_belongs_to_many relation.to_sym end + before_destroy { relationships.clear } + default_scope { order("created_at desc") } + def relationships + associations = [] + ON_MODELS.each do |association_name, _reflection| + associations << self.send("#{association_name}").to_a + end + associations.flatten! + end + # 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 - end + self.destroy unless relationships.size > 0 end # This is split into a side-effect free method and a side-effecting method diff --git a/app/models/planting.rb b/app/models/planting.rb index 8dc339687..6e869be33 100644 --- a/app/models/planting.rb +++ b/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) } diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 5ec32bf73..b1f4f2759 100644 --- a/spec/models/photo_spec.rb +++ b/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 From 77c64d59258b7692231a7bf21b63570be9bf5359 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Tue, 29 Nov 2016 13:51:04 +0000 Subject: [PATCH 21/97] Test PlantingHelper.display_days_before_maturity This ensures that #1079 is fixed. --- spec/helpers/plantings_helper_spec.rb | 63 +++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb index 9485af81d..48a1ea5ad 100644 --- a/spec/helpers/plantings_helper_spec.rb +++ b/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 16 + 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 4 + end + end + describe "display_planting" do let!(:member) { FactoryGirl.build(:member, login_name: 'crop_lady') } From b0b864a5d41278e6695c003a5114425f7ca25d2d Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Tue, 29 Nov 2016 13:53:26 +0000 Subject: [PATCH 22/97] Fix off-by-one bug in display_days_before_maturity We were counting from DateTime.now rather than Date.current, causing us to under-count by a day. --- app/helpers/plantings_helper.rb | 4 ++-- spec/helpers/plantings_helper_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/helpers/plantings_helper.rb b/app/helpers/plantings_helper.rb index a9cfebf61..964b8fb76 100644 --- a/app/helpers/plantings_helper.rb +++ b/app/helpers/plantings_helper.rb @@ -3,11 +3,11 @@ module PlantingsHelper if planting.finished? 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 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 end end diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb index 48a1ea5ad..2370bf53a 100644 --- a/spec/helpers/plantings_helper_spec.rb +++ b/spec/helpers/plantings_helper_spec.rb @@ -21,7 +21,7 @@ describe PlantingsHelper do days_before_maturity: 17 ) result = helper.display_days_before_maturity(planting) - expect(result).to eq 16 + expect(result).to eq 17 end it "handles completed plantings" do @@ -60,7 +60,7 @@ describe PlantingsHelper do days_before_maturity: nil ) result = helper.display_days_before_maturity(planting) - expect(result).to eq 4 + expect(result).to eq 5 end end From 9400225f65b283a6bf1ff4bca5dc1a9dcb87e8a3 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Wed, 30 Nov 2016 09:56:31 +0000 Subject: [PATCH 23/97] Return a string from display_days_before_maturity Sometimes we were returning a string, and sometimes we were returning an integer. We're only ever displaying the result, and this seems a little more consistent. --- app/helpers/plantings_helper.rb | 6 +++--- spec/helpers/plantings_helper_spec.rb | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/helpers/plantings_helper.rb b/app/helpers/plantings_helper.rb index 964b8fb76..f4d8d45be 100644 --- a/app/helpers/plantings_helper.rb +++ b/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 - Date.current).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) - Date.current).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 diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb index 2370bf53a..e776501b4 100644 --- a/spec/helpers/plantings_helper_spec.rb +++ b/spec/helpers/plantings_helper_spec.rb @@ -21,13 +21,13 @@ describe PlantingsHelper do days_before_maturity: 17 ) result = helper.display_days_before_maturity(planting) - expect(result).to eq 17 + 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 + expect(result).to eq "0" end it "handles plantings that should have finished" do @@ -35,10 +35,10 @@ describe PlantingsHelper do quantity: 5, planted_at: Date.new(0, 1, 1), finished_at: nil, - days_before_maturity: 17 + days_before_maturity: "17" ) result = helper.display_days_before_maturity(planting) - expect(result).to eq 0 + expect(result).to eq "0" end it "handles nil d_b_m and nil finished_at" do @@ -60,7 +60,7 @@ describe PlantingsHelper do days_before_maturity: nil ) result = helper.display_days_before_maturity(planting) - expect(result).to eq 5 + expect(result).to eq "5" end end From aa7ca71e5df3c25d98b03032cb68a8a8d0380716 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Wed, 30 Nov 2016 09:47:48 -0500 Subject: [PATCH 24/97] rubocop --- app/models/concerns/photo_capable.rb | 8 +++----- app/models/harvest.rb | 1 - app/models/photo.rb | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb index f2f53aa42..4ee0d2756 100644 --- a/app/models/concerns/photo_capable.rb +++ b/app/models/concerns/photo_capable.rb @@ -8,11 +8,9 @@ module PhotoCapable end def remove_from_list - photolist = self.photos.to_a # save a temp copy of the photo list - self.photos.clear # clear relationship b/w object and photo + photolist = photos.to_a # save a temp copy of the photo list + photos.clear # clear relationship b/w object and photo - photolist.each do |photo| - photo.destroy_if_unused - end + photolist.each(&:destroy_if_unused) end end diff --git a/app/models/harvest.rb b/app/models/harvest.rb index 1afdf473d..e7304802a 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -8,7 +8,6 @@ class Harvest < ActiveRecord::Base belongs_to :owner, class_name: 'Member' belongs_to :plant_part - default_scope { order('created_at DESC') } validates :crop, approved: true diff --git a/app/models/photo.rb b/app/models/photo.rb index bd41b5c39..eef1d4e94 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -1,5 +1,5 @@ class Photo < ActiveRecord::Base - ON_MODELS = %w(plantings harvests gardens) + ON_MODELS = %w(plantings harvests gardens).freeze belongs_to :owner, class_name: 'Member' ON_MODELS.each do |relation| @@ -13,7 +13,7 @@ class Photo < ActiveRecord::Base def relationships associations = [] ON_MODELS.each do |association_name, _reflection| - associations << self.send("#{association_name}").to_a + associations << self.send(association_name.to_s).to_a end associations.flatten! end From 6321d1ac41ddf329e495bcb8c38803c5093b927b Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 10:17:22 +1300 Subject: [PATCH 25/97] current order, use rails' findby --- app/models/member.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/member.rb b/app/models/member.rb index 018860365..da3c50271 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -107,7 +107,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 From cef1bb10567d02e1e008ed0a8b1914d3988620c0 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Wed, 30 Nov 2016 16:29:02 -0500 Subject: [PATCH 26/97] ignore has_and_belongs_to_many --- app/models/concerns/photo_capable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb index 4ee0d2756..7c5207a72 100644 --- a/app/models/concerns/photo_capable.rb +++ b/app/models/concerns/photo_capable.rb @@ -2,7 +2,7 @@ module PhotoCapable extend ActiveSupport::Concern included do - has_and_belongs_to_many :photos + has_and_belongs_to_many :photos # rubocop:disable Rails/HasAndBelongsToMany before_destroy :remove_from_list end From cd0e287dbad1c37d1d8dbec86206006ca3137cf6 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 12:50:16 +1300 Subject: [PATCH 27/97] removed member look up that wasn't used in spec --- spec/features/unsubscribing_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/features/unsubscribing_spec.rb b/spec/features/unsubscribing_spec.rb index d5da11160..8244d207a 100644 --- a/spec/features/unsubscribing_spec.rb +++ b/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" - Member.find(member.id) # reload the member expect(member.send_planting_reminder).to eq(true) expect(member.send_notification_email).to eq(true) end From e06d110861d32b2843262df1b7d175fcaac41a85 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 13:30:00 +1300 Subject: [PATCH 28/97] Turn on rails:true in rubocop so we don't need to pass --rails --- .rubocop.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index d2bb99356..44a1e816c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -8,6 +8,9 @@ AllCops: - 'db/schema.rb' - 'vendor/**/*' +Rails: + Enabled: true + Style/FileName: Exclude: - 'Gemfile' From 31bbf42ad0f652d6bb9203f1ee7badf03803da93 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 00:19:33 -0500 Subject: [PATCH 29/97] define valid models for photos in a constants file remove all hardcoded model names from photo.rb and photos_controller.rb --- app/constants/photo_models.rb | 37 ++++++++++++++++++++++++++++ app/controllers/photos_controller.rb | 35 ++++++-------------------- app/models/photo.rb | 12 ++++----- 3 files changed, 50 insertions(+), 34 deletions(-) create mode 100644 app/constants/photo_models.rb diff --git a/app/constants/photo_models.rb b/app/constants/photo_models.rb new file mode 100644 index 000000000..bc9fc24fd --- /dev/null +++ b/app/constants/photo_models.rb @@ -0,0 +1,37 @@ +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 + + ALL = [PLANTING, HARVEST, GARDEN].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 diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index ba2d24cee..95674534d 100644 --- a/app/controllers/photos_controller.rb +++ b/app/controllers/photos_controller.rb @@ -113,36 +113,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 diff --git a/app/models/photo.rb b/app/models/photo.rb index eef1d4e94..51489572f 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -1,26 +1,24 @@ class Photo < ActiveRecord::Base - ON_MODELS = %w(plantings harvests gardens).freeze belongs_to :owner, class_name: 'Member' - ON_MODELS.each do |relation| + Growstuff::Constants::PhotoModels::relations.each do |relation| has_and_belongs_to_many relation.to_sym end - before_destroy { relationships.clear } + before_destroy { all_associations.clear } default_scope { order("created_at desc") } - def relationships + def all_associations associations = [] - ON_MODELS.each do |association_name, _reflection| + Growstuff::Constants::PhotoModels::relations.each do |association_name| associations << self.send(association_name.to_s).to_a end associations.flatten! end - # remove photos that aren't used by anything def destroy_if_unused - self.destroy unless relationships.size > 0 + self.destroy unless all_associations.size > 0 end # This is split into a side-effect free method and a side-effecting method From 7b2be73c88547ffc5e381190617530e8eec57780 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 00:22:52 -0500 Subject: [PATCH 30/97] code climate --- app/controllers/photos_controller.rb | 6 +++--- app/models/photo.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 95674534d..07274b97b 100644 --- a/app/controllers/photos_controller.rb +++ b/app/controllers/photos_controller.rb @@ -114,11 +114,11 @@ class PhotosController < ApplicationController end def add_photo_to_collection - raise "Missing or invalid type provided" unless Growstuff::Constants::PhotoModels::types.include?(params[:type]) + 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]) + collection = Growstuff::Constants::PhotoModels.get_relation(@photo, params[:type]) - item_class = Growstuff::Constants::PhotoModels::get_item(params[:type]) + 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 diff --git a/app/models/photo.rb b/app/models/photo.rb index 51489572f..d1b0cd51c 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -1,7 +1,7 @@ class Photo < ActiveRecord::Base belongs_to :owner, class_name: 'Member' - Growstuff::Constants::PhotoModels::relations.each do |relation| + Growstuff::Constants::PhotoModels.relations.each do |relation| has_and_belongs_to_many relation.to_sym end @@ -11,7 +11,7 @@ class Photo < ActiveRecord::Base def all_associations associations = [] - Growstuff::Constants::PhotoModels::relations.each do |association_name| + Growstuff::Constants::PhotoModels.relations.each do |association_name| associations << self.send(association_name.to_s).to_a end associations.flatten! From ae26e3f9364b15242791ebe3c03757a11edfd38e Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 20:14:24 +1300 Subject: [PATCH 31/97] Moved login_name_or_email query to own method --- app/models/member.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/member.rb b/app/models/member.rb index 3a542c8d0..045eb26aa 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -92,7 +92,7 @@ class Member < ActiveRecord::Base 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 + where(conditions).login_name_or_email(login).first else where(conditions).first end @@ -196,6 +196,10 @@ class Member < ActiveRecord::Base return false end + def Member.login_name_or_email(login) + where(["lower(login_name) = :value OR lower(email) = :value", { value: login.downcase }]) + end + def Member.interesting howmany = 12 # max number to find interesting_members = [] From 37adbe5f4807d6a712d7d0c8c57dfceb783b77ee Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 20:16:45 +1300 Subject: [PATCH 32/97] Moved case-insensitve login look up to method --- app/models/member.rb | 4 ++++ lib/haml/filters/growstuff_markdown.rb | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/member.rb b/app/models/member.rb index 045eb26aa..2e746de3f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -200,6 +200,10 @@ class Member < ActiveRecord::Base 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 howmany = 12 # max number to find interesting_members = [] diff --git a/lib/haml/filters/growstuff_markdown.rb b/lib/haml/filters/growstuff_markdown.rb index ebd3ecebe..28e6b2543 100644 --- a/lib/haml/filters/growstuff_markdown.rb +++ b/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})" From cba02ae05c2b76ae9ae2e44ee0b70b57beaaf023 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 20:18:57 +1300 Subject: [PATCH 33/97] Convert where.first to find_by --- app/models/member.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/member.rb b/app/models/member.rb index 2e746de3f..e0c42d78d 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -266,6 +266,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 From dc504fe36347a9e704a287a8845990cf444617c5 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 21:29:00 +1300 Subject: [PATCH 34/97] use Member.case_insensitive_login_name --- app/models/post.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index ed4bcad73..8a071d166 100644 --- a/app/models/post.rb +++ b/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 From aba06c1fafeb0477a6a5e6763e2c1468418f0d91 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 21:29:28 +1300 Subject: [PATCH 35/97] Case insensitive crop look up --- app/models/crop.rb | 4 ++++ app/models/post.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index db44bbbff..2e9da34a0 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -330,6 +330,10 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength 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) diff --git a/app/models/post.rb b/app/models/post.rb index 8a071d166..bca9bb696 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -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 From d2fb96b3d7b080c3a1bb252d74ac2c90fe490143 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 1 Dec 2016 22:14:50 +1300 Subject: [PATCH 36/97] change where.first -> find_by --- app/models/member.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/member.rb b/app/models/member.rb index e0c42d78d..d79a6994f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -94,7 +94,7 @@ class Member < ActiveRecord::Base if login = conditions.delete(:login) where(conditions).login_name_or_email(login).first else - where(conditions).first + find_by(conditions) end end From 3d845f47b9d375a02f542f1194bc0767d078c31f Mon Sep 17 00:00:00 2001 From: Cesy Avon Date: Thu, 1 Dec 2016 09:42:55 +0000 Subject: [PATCH 37/97] Update rails --- Gemfile | 2 +- Gemfile.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index e752f553a..93999e0ab 100644 --- a/Gemfile +++ b/Gemfile @@ -3,7 +3,7 @@ source 'https://rubygems.org' ruby '2.3.1' -gem 'rails', '~> 4.2.1' +gem 'rails', '~> 4.2.7' gem 'bundler', '>=1.1.5' diff --git a/Gemfile.lock b/Gemfile.lock index 618de9759..f7aaaf9c8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -544,7 +544,7 @@ DEPENDENCIES poltergeist pry quiet_assets - rails (~> 4.2.1) + rails (~> 4.2.7) rails_12factor rake (>= 10.0.0) rspec-activemodel-mocks From 54628e6d8c8bab66a939a69c77099f7273165c3f Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 12:24:26 -0500 Subject: [PATCH 38/97] expand wikipedia regex to include punctuation, because some have - or () in name (Fixes: #1104) --- app/models/crop.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index 07932b1b6..dbf81f29f 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -42,7 +42,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength ## Wikipedia urls are only necessary when approving a crop validates :en_wikipedia_url, format: { - with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:]%_]+\z/, + with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:][:punct:]]+\z/, message: 'is not a valid English Wikipedia URL' }, if: :approved? From b38945d62f6a92d78347c25013736d91f589af3a Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 12:53:45 -0500 Subject: [PATCH 39/97] add require_relative to photo_capable concern for constants starts #1107 --- app/models/concerns/photo_capable.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb index 7c5207a72..61c92c98d 100644 --- a/app/models/concerns/photo_capable.rb +++ b/app/models/concerns/photo_capable.rb @@ -1,3 +1,4 @@ +require_relative '../../constants/photo_models.rb' module PhotoCapable extend ActiveSupport::Concern From 637b46ef1088a9dd49b3b11c8cff16fafc9d3c09 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 12:54:55 -0500 Subject: [PATCH 40/97] add photos to seeds Fixes #495 --- app/constants/photo_models.rb | 3 ++- app/models/seed.rb | 1 + db/schema.rb | 15 ++++++++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/constants/photo_models.rb b/app/constants/photo_models.rb index bc9fc24fd..8f79933e4 100644 --- a/app/constants/photo_models.rb +++ b/app/constants/photo_models.rb @@ -4,8 +4,9 @@ module Growstuff 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].freeze + ALL = [PLANTING, HARVEST, GARDEN, SEED].freeze def self.types ALL.map do |model| diff --git a/app/models/seed.rb b/app/models/seed.rb index 4c8a1c1c7..02a36fe3a 100644 --- a/app/models/seed.rb +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 6596b2428..4901b8499 100644 --- a/db/schema.rb +++ b/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" @@ -355,6 +355,14 @@ ActiveRecord::Schema.define(version: 20161129021533) do t.integer "product_id" end + create_table "photo_mappings", force: :cascade do |t| + t.integer "photo_id" + t.integer "object_id" + t.string "object_type" + end + + add_index "photo_mappings", ["object_id", "photo_id"], name: "index_photo_mappings_on_object_id_and_photo_id", using: :btree + create_table "photos", force: :cascade do |t| t.integer "owner_id", null: false t.string "thumbnail_url", null: false @@ -373,6 +381,11 @@ 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 + create_table "plant_parts", force: :cascade do |t| t.string "name" t.datetime "created_at" From a3c8bc0586f68b0457dc2acc5d7c7822b0f4b5bf Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 13:23:52 -0500 Subject: [PATCH 41/97] include the migration --- db/migrate/20161201154922_add_photos_seeds_table.rb | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 db/migrate/20161201154922_add_photos_seeds_table.rb diff --git a/db/migrate/20161201154922_add_photos_seeds_table.rb b/db/migrate/20161201154922_add_photos_seeds_table.rb new file mode 100644 index 000000000..12605ae4c --- /dev/null +++ b/db/migrate/20161201154922_add_photos_seeds_table.rb @@ -0,0 +1,8 @@ +class AddPhotosSeedsTable < ActiveRecord::Migration + def change + create_table :photos_seeds, id: false do |t| + t.integer :photo_id + t.integer :seed_id + end + end +end From 2fb34bea182b8e7592c617f9ee208d7a87cc6091 Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 13:39:43 -0500 Subject: [PATCH 42/97] narrow the regex back down hyphen must be the LAST thing in the character class --- app/models/crop.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index dbf81f29f..71fd2205d 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -42,7 +42,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength ## Wikipedia urls are only necessary when approving a crop validates :en_wikipedia_url, format: { - with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:][:punct:]]+\z/, + with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:]%_()-]+\z/, message: 'is not a valid English Wikipedia URL' }, if: :approved? From ceac906a3f2787346512a56a2699baaa31fbf1eb Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Thu, 1 Dec 2016 14:00:18 -0500 Subject: [PATCH 43/97] ignore the minified bootstrap accessibility file for code climate checks --- .codeclimate.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.codeclimate.yml b/.codeclimate.yml index 3a369a376..e1c13452a 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -38,3 +38,4 @@ exclude_paths: - db/ - spec/ - public/ +- app/assets/stylesheets/bootstrap-accessibility.css From 03cb4a8deea4c09712f94afa6efca156b4c71357 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 2 Dec 2016 00:23:46 +0000 Subject: [PATCH 44/97] use find_or_initialize_by --- app/controllers/alternate_names_controller.rb | 2 +- app/controllers/harvests_controller.rb | 2 +- app/controllers/scientific_names_controller.rb | 2 +- app/controllers/seeds_controller.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/alternate_names_controller.rb b/app/controllers/alternate_names_controller.rb index ef81497e1..b8467bb34 100644 --- a/app/controllers/alternate_names_controller.rb +++ b/app/controllers/alternate_names_controller.rb @@ -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 diff --git a/app/controllers/harvests_controller.rb b/app/controllers/harvests_controller.rb index 3d36438fd..aa80c21c7 100644 --- a/app/controllers/harvests_controller.rb +++ b/app/controllers/harvests_controller.rb @@ -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 diff --git a/app/controllers/scientific_names_controller.rb b/app/controllers/scientific_names_controller.rb index 35dbb4e8a..0fd0eccac 100644 --- a/app/controllers/scientific_names_controller.rb +++ b/app/controllers/scientific_names_controller.rb @@ -28,7 +28,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 diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb index 5d9a775e7..01fdc7beb 100644 --- a/app/controllers/seeds_controller.rb +++ b/app/controllers/seeds_controller.rb @@ -49,7 +49,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 From 81f2fa5fa4a7a4ee53c883855470498c70558af5 Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 19 Sep 2015 16:24:01 -0400 Subject: [PATCH 45/97] GS-658: sort locale keys, add rake task for it --- config/locales/en.yml | 3 +++ config/locales/ja.yml | 1 + lib/tasks/i18n.rake | 6 ++++++ 3 files changed, 10 insertions(+) create mode 100644 lib/tasks/i18n.rake diff --git a/config/locales/en.yml b/config/locales/en.yml index bb9058e53..2c73ad544 100644 --- a/config/locales/en.yml +++ b/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: diff --git a/config/locales/ja.yml b/config/locales/ja.yml index fa240b7c3..2a51c32c5 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -1,3 +1,4 @@ +--- ja: home: blurb: diff --git a/lib/tasks/i18n.rake b/lib/tasks/i18n.rake new file mode 100644 index 000000000..447f0a5d3 --- /dev/null +++ b/lib/tasks/i18n.rake @@ -0,0 +1,6 @@ +namespace :i18n do + desc "sort all locale keys" + task normalize: :environment do + `i18n-tasks normalize` + end +end From fe6e269c64e5cdc6cb6efd142f49c2098d820c90 Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 19 Sep 2015 17:45:31 -0400 Subject: [PATCH 46/97] GS-658 - i18n automation POC --- Gemfile | 1 + Gemfile.lock | 1 + app/views/layouts/_header.html.haml | 14 ++-- .../layouts/_header.html.i18n-extractor.haml | 78 +++++++++++++++++++ config/locales/en.yml | 32 ++++++-- lib/tasks/i18n.rake | 16 +++- 6 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 app/views/layouts/_header.html.i18n-extractor.haml diff --git a/Gemfile b/Gemfile index 93999e0ab..a91ccba45 100644 --- a/Gemfile +++ b/Gemfile @@ -113,6 +113,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 diff --git a/Gemfile.lock b/Gemfile.lock index f7aaaf9c8..2701ee162 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -524,6 +524,7 @@ DEPENDENCIES guard guard-rspec haml + haml-i18n-extractor haml-rails heroku-api i18n-tasks diff --git a/app/views/layouts/_header.html.haml b/app/views/layouts/_header.html.haml index 5b66c20dc..803d542f5 100644 --- a/app/views/layouts/_header.html.haml +++ b/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 diff --git a/app/views/layouts/_header.html.i18n-extractor.haml b/app/views/layouts/_header.html.i18n-extractor.haml new file mode 100644 index 000000000..5523e75fe --- /dev/null +++ b/app/views/layouts/_header.html.i18n-extractor.haml @@ -0,0 +1,78 @@ +.sr-only + =link_to t(".skip"), "#skipnav" +.navbar.navbar-default.navbar-fixed-top(role="navigation") + .container + .navbar-header + %button.navbar-toggle(data-target="#navbar-collapse" data-toggle="collapse") + %span.sr-only= t('.toggle_navigation') + %span.icon-bar + %span.icon-bar + %span.icon-bar + %a.navbar-brand(href=root_path) + = image_tag("growstuff-brand.png", :size => "200x50", :alt => ENV['GROWSTUFF_SITE_NAME']) + = form_tag crops_search_path, :method => :get, :id => 'navbar-search', :class => 'navbar-form pull-right' do + .input + = label_tag :term, "Search crop database:", :class => 'sr-only' + = text_field_tag 'term', nil, :class => 'search-query input-medium form-control', :placeholder => 'Search crops' + = submit_tag "Search", :class => 'btn sr-only' + + .navbar-collapse.collapse#navbar-collapse + %ul.nav.navbar-nav.pull-right + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => crops_path} + = t('.crops') + %b.caret + %ul.dropdown-menu + %li= link_to t('.browse_crops'), crops_path + %li= link_to t('.seeds'), seeds_path + %li= link_to t('.plantings'), plantings_path + %li= link_to t('.harvests'), harvests_path + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => members_path} + = t('.community') + %b.caret + %ul.dropdown-menu + %li= link_to t('.community_map'), places_path + %li= link_to t('.browse_members'), members_path + %li= link_to t('.posts'), posts_path + %li= link_to t('.forums'), forums_path + %li= link_to t('.support_growstuff'), shop_path + + - if member_signed_in? + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => root_path} + - if current_member.notifications.unread_count > 0 + = t('.your_stuff', unread_count: current_member.notifications.unread_count) + - else + = 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) + %li= link_to t('.gardens'), gardens_by_owner_path(:owner => current_member.slug) + %li= link_to t('.plantings'), plantings_by_owner_path(:owner => current_member.slug) + %li= link_to t('.harvest'), harvests_by_owner_path(:owner => current_member.slug) + %li= link_to t('.seeds'), seeds_by_owner_path(:owner => current_member.slug) + %li= link_to t('.posts'), posts_by_author_path(:author => current_member.slug) + %li= link_to t('.account'), orders_path + %li + - if current_member.notifications.unread_count > 0 + = link_to(t('.inbox_unread', unread_count: current_member.notifications.unread_count), notifications_path) + - else + = link_to(t('.inbox'), notifications_path) + - if current_member.has_role?(:crop_wrangler) || current_member.has_role?(:admin) + %li{:class => 'divider', :role => 'presentation'} + - if current_member.has_role?(:crop_wrangler) + %li= link_to t('.crop_wrangling'), wrangle_crops_path + - if current_member.has_role?(:admin) + %li= link_to t('.admin'), admin_path + + + %li= link_to t('.sign_out'), destroy_member_session_path, :method => :delete + + - else + %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 +%a{:name => 'skipnav'} diff --git a/config/locales/en.yml b/config/locales/en.yml index 2c73ad544..1acb4e35c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -56,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 @@ -84,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 @@ -108,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" @@ -132,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" diff --git a/lib/tasks/i18n.rake b/lib/tasks/i18n.rake index 447f0a5d3..917cd73f6 100644 --- a/lib/tasks/i18n.rake +++ b/lib/tasks/i18n.rake @@ -1,6 +1,18 @@ namespace :i18n do - desc "sort all locale keys" - task normalize: :environment 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 From 160c6efd04d2b2d54dc9913661869ee2991e9417 Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 26 Sep 2015 11:54:02 -0400 Subject: [PATCH 47/97] Remove example file, add documentation --- .../layouts/_header.html.i18n-extractor.haml | 78 ------------------- config/locales/README.md | 15 ++++ 2 files changed, 15 insertions(+), 78 deletions(-) delete mode 100644 app/views/layouts/_header.html.i18n-extractor.haml create mode 100644 config/locales/README.md diff --git a/app/views/layouts/_header.html.i18n-extractor.haml b/app/views/layouts/_header.html.i18n-extractor.haml deleted file mode 100644 index 5523e75fe..000000000 --- a/app/views/layouts/_header.html.i18n-extractor.haml +++ /dev/null @@ -1,78 +0,0 @@ -.sr-only - =link_to t(".skip"), "#skipnav" -.navbar.navbar-default.navbar-fixed-top(role="navigation") - .container - .navbar-header - %button.navbar-toggle(data-target="#navbar-collapse" data-toggle="collapse") - %span.sr-only= t('.toggle_navigation') - %span.icon-bar - %span.icon-bar - %span.icon-bar - %a.navbar-brand(href=root_path) - = image_tag("growstuff-brand.png", :size => "200x50", :alt => ENV['GROWSTUFF_SITE_NAME']) - = form_tag crops_search_path, :method => :get, :id => 'navbar-search', :class => 'navbar-form pull-right' do - .input - = label_tag :term, "Search crop database:", :class => 'sr-only' - = text_field_tag 'term', nil, :class => 'search-query input-medium form-control', :placeholder => 'Search crops' - = submit_tag "Search", :class => 'btn sr-only' - - .navbar-collapse.collapse#navbar-collapse - %ul.nav.navbar-nav.pull-right - %li.dropdown< - %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => crops_path} - = t('.crops') - %b.caret - %ul.dropdown-menu - %li= link_to t('.browse_crops'), crops_path - %li= link_to t('.seeds'), seeds_path - %li= link_to t('.plantings'), plantings_path - %li= link_to t('.harvests'), harvests_path - %li.dropdown< - %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => members_path} - = t('.community') - %b.caret - %ul.dropdown-menu - %li= link_to t('.community_map'), places_path - %li= link_to t('.browse_members'), members_path - %li= link_to t('.posts'), posts_path - %li= link_to t('.forums'), forums_path - %li= link_to t('.support_growstuff'), shop_path - - - if member_signed_in? - %li.dropdown< - %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => root_path} - - if current_member.notifications.unread_count > 0 - = t('.your_stuff', unread_count: current_member.notifications.unread_count) - - else - = 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) - %li= link_to t('.gardens'), gardens_by_owner_path(:owner => current_member.slug) - %li= link_to t('.plantings'), plantings_by_owner_path(:owner => current_member.slug) - %li= link_to t('.harvest'), harvests_by_owner_path(:owner => current_member.slug) - %li= link_to t('.seeds'), seeds_by_owner_path(:owner => current_member.slug) - %li= link_to t('.posts'), posts_by_author_path(:author => current_member.slug) - %li= link_to t('.account'), orders_path - %li - - if current_member.notifications.unread_count > 0 - = link_to(t('.inbox_unread', unread_count: current_member.notifications.unread_count), notifications_path) - - else - = link_to(t('.inbox'), notifications_path) - - if current_member.has_role?(:crop_wrangler) || current_member.has_role?(:admin) - %li{:class => 'divider', :role => 'presentation'} - - if current_member.has_role?(:crop_wrangler) - %li= link_to t('.crop_wrangling'), wrangle_crops_path - - if current_member.has_role?(:admin) - %li= link_to t('.admin'), admin_path - - - %li= link_to t('.sign_out'), destroy_member_session_path, :method => :delete - - - else - %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 -%a{:name => 'skipnav'} diff --git a/config/locales/README.md b/config/locales/README.md new file mode 100644 index 000000000..5ce22aa89 --- /dev/null +++ b/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 From e939be05f86d270fd8d26dd468de652bb16c091a Mon Sep 17 00:00:00 2001 From: Jim Stallings Date: Sat, 26 Sep 2015 11:54:51 -0400 Subject: [PATCH 48/97] Add myself to contributors --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index f4c22eccf..67c9b0b7c 100644 --- a/CONTRIBUTORS.md +++ b/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) From aa638b8a68c8af120581d2463067c86a4ec0639c Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 2 Dec 2016 15:41:35 +1030 Subject: [PATCH 49/97] Missed from merge --- Gemfile.lock | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a3886ab62..425491475 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -204,17 +204,13 @@ GEM rspec (>= 2.99.0, < 4.0) haml (4.0.7) tilt -<<<<<<< HEAD - haml-rails (0.9.0) -======= haml-i18n-extractor (0.5.9) activesupport haml highline tilt trollop (= 1.16.2) - haml-rails (0.6.0) ->>>>>>> 2f561aaa473120cceb88be3be21fcc657cc7729e + haml-rails (0.9.0) actionpack (>= 4.0.1) activesupport (>= 4.0.1) haml (>= 4.0.6, < 5.0) @@ -468,14 +464,9 @@ GEM thor (0.19.3) thread (0.2.2) thread_safe (0.3.5) -<<<<<<< HEAD tilt (2.0.5) tins (1.13.0) -======= - tilt (1.4.1) - tins (1.3.3) trollop (1.16.2) ->>>>>>> 2f561aaa473120cceb88be3be21fcc657cc7729e tzinfo (1.2.2) thread_safe (~> 0.1) uglifier (2.7.2) From fa65be40a402966b5f7d99127ed38cc050e7ce34 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 2 Dec 2016 15:51:29 +1030 Subject: [PATCH 50/97] lib/tasks/i18n.rake:8:37: W: Lint/UnusedBlockArgument: Unused block argument - t. If it's necessary, use _ or _t as an argument name to indicate that it won't be used. --- lib/tasks/i18n.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/i18n.rake b/lib/tasks/i18n.rake index 917cd73f6..04fa8001b 100644 --- a/lib/tasks/i18n.rake +++ b/lib/tasks/i18n.rake @@ -5,7 +5,7 @@ namespace :i18n do end desc "translate haml strings into i18 en locale using haml-i18n-extractor" - task :extractor, [:haml_path] do |t, args| + task :extractor, [:haml_path] do |_t, args| require 'haml-i18n-extractor' haml_path = args[:haml_path] begin From 7aaf6ea2ece102ec3e1be827d2a6164cadef7f0c Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 2 Dec 2016 20:21:41 +1300 Subject: [PATCH 51/97] Revert "Changed phantomjs url to project official" This reverts commit 7fcd3cba8d1471b4c3a7a50ad35eac63590764ae. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 5f2219acd..901cab8fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,7 +16,7 @@ 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; From 0c2a60ecc3361ada06ac50fec7cad6525109f879 Mon Sep 17 00:00:00 2001 From: Shiny Date: Sat, 3 Dec 2016 02:06:53 +1300 Subject: [PATCH 52/97] Reduce max AbcSize to 38 (#1112) Reduce how complex methods are allowed to be, according to our code checker, so we're forced to write readable code --- .rubocop.yml | 2 +- app/models/ability.rb | 2 +- db/migrate/20150201052245_create_cms.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 2631b0f0d..74b45768d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -34,7 +34,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. diff --git a/app/models/ability.rb b/app/models/ability.rb index 4ce056438..9085d7962 100644 --- a/app/models/ability.rb +++ b/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 diff --git a/db/migrate/20150201052245_create_cms.rb b/db/migrate/20150201052245_create_cms.rb index 2a187e66f..3e3101df0 100644 --- a/db/migrate/20150201052245_create_cms.rb +++ b/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' {} From c201200e9bd9fbc021824fb20c70767c60b2d48e Mon Sep 17 00:00:00 2001 From: Mackenzie Morgan Date: Fri, 2 Dec 2016 23:17:01 -0500 Subject: [PATCH 53/97] correct schema and add index for photos_seeds --- db/migrate/20161201154922_add_photos_seeds_table.rb | 1 + db/schema.rb | 10 ++-------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/db/migrate/20161201154922_add_photos_seeds_table.rb b/db/migrate/20161201154922_add_photos_seeds_table.rb index 12605ae4c..31693d225 100644 --- a/db/migrate/20161201154922_add_photos_seeds_table.rb +++ b/db/migrate/20161201154922_add_photos_seeds_table.rb @@ -4,5 +4,6 @@ class AddPhotosSeedsTable < ActiveRecord::Migration t.integer :photo_id t.integer :seed_id end + add_index(:photos_seeds, [:seed_id, :photo_id]) end end diff --git a/db/schema.rb b/db/schema.rb index 4901b8499..429fabf04 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -355,14 +355,6 @@ ActiveRecord::Schema.define(version: 20161201154922) do t.integer "product_id" end - create_table "photo_mappings", force: :cascade do |t| - t.integer "photo_id" - t.integer "object_id" - t.string "object_type" - end - - add_index "photo_mappings", ["object_id", "photo_id"], name: "index_photo_mappings_on_object_id_and_photo_id", using: :btree - create_table "photos", force: :cascade do |t| t.integer "owner_id", null: false t.string "thumbnail_url", null: false @@ -386,6 +378,8 @@ ActiveRecord::Schema.define(version: 20161201154922) do 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" From 92195d51d293f590a3d613db469619b4d0efc556 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sun, 4 Dec 2016 21:21:35 +0000 Subject: [PATCH 54/97] Removed redundant returns --- .rubocop_todo.yml | 26 ------------------- app/helpers/application_helper.rb | 6 ++--- app/helpers/harvests_helper.rb | 10 +++---- app/helpers/plantings_helper.rb | 8 +++--- app/mailers/notifier.rb | 2 +- app/models/account.rb | 4 +-- app/models/crop.rb | 14 +++++----- app/models/forum.rb | 2 +- app/models/garden.rb | 4 +-- app/models/harvest.rb | 4 +-- app/models/member.rb | 20 +++++++------- app/models/order.rb | 6 ++--- app/models/photo.rb | 2 +- app/models/plant_part.rb | 4 +-- app/models/planting.rb | 8 +++--- app/models/seed.rb | 8 +++--- lib/haml/filters/escaped_markdown.rb | 2 +- lib/haml/filters/growstuff_markdown.rb | 2 +- spec/controllers/accounts_controller_spec.rb | 2 +- .../haml/filters/growstuff_markdown_spec.rb | 4 +-- spec/views/devise/shared/_links_spec.rb | 2 +- 21 files changed, 55 insertions(+), 85 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 69c0c397d..cb0b978f6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -612,32 +612,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: diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0aa552bdf..1262c359f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,17 +1,17 @@ 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, + 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) diff --git a/app/helpers/harvests_helper.rb b/app/helpers/harvests_helper.rb index d848d919f..9573fcaf0 100644 --- a/app/helpers/harvests_helper.rb +++ b/app/helpers/harvests_helper.rb @@ -19,20 +19,16 @@ module HarvestsHelper 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) + 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}" + "#{number_to_human(harvest.quantity, strip_insignificant_zeros: true)} #{harvest.unit}" end - else - return nil 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 + "#{number_to_human(harvest.weight_quantity, strip_insignificant_zeros: true)} #{harvest.weight_unit}" end end diff --git a/app/helpers/plantings_helper.rb b/app/helpers/plantings_helper.rb index f4d8d45be..15f756ace 100644 --- a/app/helpers/plantings_helper.rb +++ b/app/helpers/plantings_helper.rb @@ -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 diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 281f0b6ee..69f88ba7e 100644 --- a/app/mailers/notifier.rb +++ b/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) diff --git a/app/models/account.rb b/app/models/account.rb index 449697e05..1dd914d62 100644 --- a/app/models/account.rb +++ b/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 diff --git a/app/models/crop.rb b/app/models/crop.rb index f803cfecf..2fc9a6ad1 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -172,7 +172,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength popular_plant_parts[h.plant_part] += 1 end end - return popular_plant_parts + popular_plant_parts end def interesting? @@ -180,7 +180,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 +213,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) @@ -281,9 +281,9 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength def rejection_explanation if reason_for_rejection == "other" - return rejection_notes + rejection_notes else - return reason_for_rejection + reason_for_rejection end end @@ -309,7 +309,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 @@ -326,7 +326,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength matches.unshift(exact_match) end - return matches + matches end end diff --git a/app/models/forum.rb b/app/models/forum.rb index 810653d7c..42bf1800e 100644 --- a/app/models/forum.rb +++ b/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 diff --git a/app/models/garden.rb b/app/models/garden.rb index 9f58ac0ab..8f4e73678 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -72,7 +72,7 @@ class Garden < ActiveRecord::Base end end - return unique_plantings[0..3] + unique_plantings[0..3] end def to_s @@ -91,6 +91,6 @@ class Garden < ActiveRecord::Base end def default_photo - return photos.first + photos.first end end diff --git a/app/models/harvest.rb b/app/models/harvest.rb index e7304802a..3c5b3ac96 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -117,10 +117,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 diff --git a/app/models/member.rb b/app/models/member.rb index d79a6994f..931a73402 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -99,7 +99,7 @@ class Member < ActiveRecord::Base end def to_s - return login_name + login_name end def has_role?(role_sym) @@ -128,16 +128,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,7 +152,7 @@ class Member < ActiveRecord::Base @flickr.access_secret = flickr_auth.secret end end - return @flickr + @flickr end # Fetches a collection of photos from Flickr @@ -185,7 +185,7 @@ class Member < ActiveRecord::Base flickr.photosets.getList.each do |p| sets[p.title] = p.id end - return sets + sets end def interesting? @@ -193,7 +193,7 @@ 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) @@ -213,7 +213,7 @@ class Member < ActiveRecord::Base interesting_members.push(m) end end - return interesting_members + interesting_members end def Member.nearest_to(place) @@ -224,7 +224,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 diff --git a/app/models/order.rb b/app/models/order.rb index b363f9b71..81caa7f8e 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -21,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 @@ -34,7 +34,7 @@ class Order < ActiveRecord::Base amount: i.price }) end - return items + items end # record the paypal details for reference @@ -92,6 +92,6 @@ class Order < ActiveRecord::Base return Order.where(referral_code: args[:for].upcase) end end - return [] + [] end end diff --git a/app/models/photo.rb b/app/models/photo.rb index d1b0cd51c..5785ac6f0 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -28,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, diff --git a/app/models/plant_part.rb b/app/models/plant_part.rb index 16f23822a..fdf1d69e6 100644 --- a/app/models/plant_part.rb +++ b/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 diff --git a/app/models/planting.rb b/app/models/planting.rb index 6e869be33..52c2d5463 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -68,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 @@ -77,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) @@ -136,6 +136,6 @@ class Planting < ActiveRecord::Base interesting_plantings.push(p) end - return interesting_plantings + interesting_plantings end end diff --git a/app/models/seed.rb b/app/models/seed.rb index 02a36fe3a..6f441964a 100644 --- a/app/models/seed.rb +++ b/app/models/seed.rb @@ -66,9 +66,9 @@ class Seed < ActiveRecord::Base def tradable? if self.tradable_to == 'nowhere' - return false + false else - return true + true end end @@ -76,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 @@ -92,7 +92,7 @@ class Seed < ActiveRecord::Base end end - return interesting_seeds + interesting_seeds end def seed_slug diff --git a/lib/haml/filters/escaped_markdown.rb b/lib/haml/filters/escaped_markdown.rb index 1986b905d..86345b5a2 100644 --- a/lib/haml/filters/escaped_markdown.rb +++ b/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 diff --git a/lib/haml/filters/growstuff_markdown.rb b/lib/haml/filters/growstuff_markdown.rb index 28e6b2543..e03e8c306 100644 --- a/lib/haml/filters/growstuff_markdown.rb +++ b/lib/haml/filters/growstuff_markdown.rb @@ -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 diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index da21a1ba3..66375b292 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/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 diff --git a/spec/lib/haml/filters/growstuff_markdown_spec.rb b/spec/lib/haml/filters/growstuff_markdown_spec.rb index debce3111..3ee3102f4 100644 --- a/spec/lib/haml/filters/growstuff_markdown_spec.rb +++ b/spec/lib/haml/filters/growstuff_markdown_spec.rb @@ -3,7 +3,7 @@ require 'haml/filters' require 'haml/filters/growstuff_markdown' def input_link(name) - return "[#{name}](crop)" + "[#{name}](crop)" end def output_link(crop, name = nil) @@ -16,7 +16,7 @@ def output_link(crop, name = nil) end def input_member_link(name) - return "[#{name}](member)" + "[#{name}](member)" end def output_member_link(member, name = nil) diff --git a/spec/views/devise/shared/_links_spec.rb b/spec/views/devise/shared/_links_spec.rb index c567339d6..39147113f 100644 --- a/spec/views/devise/shared/_links_spec.rb +++ b/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 From 26e5a414cfc341e6dbda08107871b537a7093f86 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sun, 4 Dec 2016 21:23:33 +0000 Subject: [PATCH 55/97] Line no longer needs wrapping --- app/helpers/application_helper.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 1262c359f..f66dfe03f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -5,8 +5,7 @@ module ApplicationHelper # 999 cents becomes 9.99 AUD -- for products/orders/etc def price_with_currency(price) - 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) From 91b0c1898e009db19ff54f5e4a19e1179f86157d Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Mon, 5 Dec 2016 03:10:22 +0000 Subject: [PATCH 56/97] Simplifying display_harvest_description --- app/helpers/harvests_helper.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/helpers/harvests_helper.rb b/app/helpers/harvests_helper.rb index 9573fcaf0..afa135b5e 100644 --- a/app/helpers/harvests_helper.rb +++ b/app/helpers/harvests_helper.rb @@ -33,10 +33,7 @@ module HarvestsHelper 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 From 8981a222ea54665d59b7eea482e3f3c9e21d7ee0 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Tue, 6 Dec 2016 00:32:20 +0000 Subject: [PATCH 57/97] Simplified display_weight --- app/helpers/harvests_helper.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/helpers/harvests_helper.rb b/app/helpers/harvests_helper.rb index afa135b5e..67f668804 100644 --- a/app/helpers/harvests_helper.rb +++ b/app/helpers/harvests_helper.rb @@ -27,9 +27,8 @@ module HarvestsHelper end def display_weight(harvest) - if !harvest.weight_quantity.blank? && harvest.weight_quantity > 0 - "#{number_to_human(harvest.weight_quantity, strip_insignificant_zeros: true)} #{harvest.weight_unit}" - 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) From 4c6f0fc929adc9afa87b025d3166d99a8510a502 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Tue, 6 Dec 2016 21:09:39 +1300 Subject: [PATCH 58/97] Fixed AmbiguousOperator --- .rubocop_todo.yml | 5 ----- spec/factories/member.rb | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cb0b978f6..80feefc90 100644 --- a/.rubocop_todo.yml +++ b/.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: diff --git a/spec/factories/member.rb b/spec/factories/member.rb index 7fb02594b..0888edabb 100644 --- a/spec/factories/member.rb +++ b/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 From abc5ac5f29387915379f96832b6fafb4851d93fc Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Tue, 6 Dec 2016 21:18:13 +1300 Subject: [PATCH 59/97] Don't use assignments in conditions --- .rubocop_todo.yml | 6 ------ app/models/member.rb | 8 +++----- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cb0b978f6..ba60a33d0 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -26,12 +26,6 @@ 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: diff --git a/app/models/member.rb b/app/models/member.rb index 931a73402..598507063 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -91,11 +91,9 @@ 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).login_name_or_email(login).first - else - find_by(conditions) - end + login = conditions.delete(:login) + return where(conditions).login_name_or_email(login).first if login + find_by(conditions) end def to_s From 464c570d99c2fe48939515dbc71784871bb846fc Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Tue, 6 Dec 2016 21:23:55 +1300 Subject: [PATCH 60/97] Fixed parentheses that are interp-ed as grouped expression --- .rubocop_todo.yml | 9 --------- app/mailers/notifier.rb | 4 ++-- spec/features/harvests/browse_harvests_spec.rb | 4 ++-- spec/models/follow_spec.rb | 2 +- spec/models/member_spec.rb | 4 ++-- spec/views/plantings/show.html.haml_spec.rb | 2 +- 6 files changed, 8 insertions(+), 17 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cb0b978f6..2b9bfe57b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -37,15 +37,6 @@ 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. diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 69f88ba7e..89a174c52 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -16,7 +16,7 @@ class Notifier < ActionMailer::Base @reply_link = reply_link(@notification) # Encrypting - @signed_message = verifier.generate ({ member_id: @notification.recipient.id, type: :send_notification_email }) + @signed_message = verifier.generate(member_id: @notification.recipient.id, type: :send_notification_email) mail(to: @notification.recipient.email, subject: @notification.subject) @@ -29,7 +29,7 @@ class Notifier < ActionMailer::Base @harvests = @member.harvests.first(5) # Encrypting - @signed_message = verifier.generate ({ member_id: @member.id, type: :send_planting_reminder }) + @signed_message = verifier.generate(member_id: @member.id, type: :send_planting_reminder) if @member.send_planting_reminder mail(to: @member.email, diff --git a/spec/features/harvests/browse_harvests_spec.rb b/spec/features/harvests/browse_harvests_spec.rb index d103ec3c9..2a8400c2e 100644 --- a/spec/features/harvests/browse_harvests_spec.rb +++ b/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 diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb index 2fd0b6b80..95b82d8a8 100644 --- a/spec/models/follow_spec.rb +++ b/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 diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 19e0a9568..d5c9fb9b1 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -354,11 +354,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 diff --git a/spec/views/plantings/show.html.haml_spec.rb b/spec/views/plantings/show.html.haml_spec.rb index e89d6bb92..f6136f8f6 100644 --- a/spec/views/plantings/show.html.haml_spec.rb +++ b/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) From a7bafafa06a99842bd49639cd4bfa0fcf08f5428 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Tue, 6 Dec 2016 21:58:17 +1300 Subject: [PATCH 61/97] Yet another wikipedia url char added to regex --- app/models/crop.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index 2fc9a6ad1..8d14f981c 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -42,7 +42,7 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength ## Wikipedia urls are only necessary when approving a crop validates :en_wikipedia_url, format: { - with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:]%_()-]+\z/, + with: /\Ahttps?:\/\/en\.wikipedia\.org\/wiki\/[[:alnum:]%_\.()-]+\z/, message: 'is not a valid English Wikipedia URL' }, if: :approved? From 05e7a277823765af0d7fa3247d45d5071e179a19 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 7 Dec 2016 10:02:02 +1300 Subject: [PATCH 62/97] Call to verify options stays as a hash --- app/mailers/notifier.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 89a174c52..e97fedcda 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -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,7 +30,8 @@ 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, From e695d5646a53cfcd0748cb0e38883b63c4dcb138 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 7 Dec 2016 20:12:12 +1300 Subject: [PATCH 63/97] FIX Gardens query was running twice in contoller --- app/controllers/gardens_controller.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/gardens_controller.rb b/app/controllers/gardens_controller.rb index a83089844..d836b0e85 100644 --- a/app/controllers/gardens_controller.rb +++ b/app/controllers/gardens_controller.rb @@ -5,11 +5,12 @@ class GardensController < ApplicationController # 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 + @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 From 4f465d808c63c12777ffef5e161d614e610eb424 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 7 Dec 2016 23:29:27 +0000 Subject: [PATCH 64/97] We don't need to load the resource again --- app/controllers/account_types_controller.rb | 6 ------ app/controllers/accounts_controller.rb | 5 ----- app/controllers/alternate_names_controller.rb | 4 ---- app/controllers/comments_controller.rb | 4 ---- app/controllers/crops_controller.rb | 4 ---- app/controllers/gardens_controller.rb | 6 ------ app/controllers/harvests_controller.rb | 4 ---- app/controllers/notifications_controller.rb | 2 -- app/controllers/orders_controller.rb | 8 -------- app/controllers/photos_controller.rb | 4 ---- app/controllers/plant_parts_controller.rb | 6 ------ app/controllers/plantings_controller.rb | 4 ---- app/controllers/posts_controller.rb | 4 ---- app/controllers/products_controller.rb | 6 ------ app/controllers/roles_controller.rb | 6 ------ app/controllers/scientific_names_controller.rb | 6 ------ app/controllers/seeds_controller.rb | 6 ------ 17 files changed, 85 deletions(-) diff --git a/app/controllers/account_types_controller.rb b/app/controllers/account_types_controller.rb index fce71380a..f1c78706b 100644 --- a/app/controllers/account_types_controller.rb +++ b/app/controllers/account_types_controller.rb @@ -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| diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index bca64597a..c7396bfce 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -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.' } diff --git a/app/controllers/alternate_names_controller.rb b/app/controllers/alternate_names_controller.rb index b8467bb34..43d76fff3 100644 --- a/app/controllers/alternate_names_controller.rb +++ b/app/controllers/alternate_names_controller.rb @@ -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 diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index ef3225bfb..bde0876e8 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -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 diff --git a/app/controllers/crops_controller.rb b/app/controllers/crops_controller.rb index 111b0390b..0b9d713a0 100644 --- a/app/controllers/crops_controller.rb +++ b/app/controllers/crops_controller.rb @@ -110,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 @@ -155,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" @@ -184,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| diff --git a/app/controllers/gardens_controller.rb b/app/controllers/gardens_controller.rb index d836b0e85..fca489384 100644 --- a/app/controllers/gardens_controller.rb +++ b/app/controllers/gardens_controller.rb @@ -21,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 } @@ -42,7 +40,6 @@ class GardensController < ApplicationController # GET /gardens/1/edit def edit - @garden = Garden.find(params[:id]) end # POST /gardens @@ -66,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.' } @@ -82,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") diff --git a/app/controllers/harvests_controller.rb b/app/controllers/harvests_controller.rb index aa80c21c7..1b924cace 100644 --- a/app/controllers/harvests_controller.rb +++ b/app/controllers/harvests_controller.rb @@ -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| diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index dbc70f439..03b0ef8f8 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -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) @@ -54,7 +53,6 @@ class NotificationsController < ApplicationController # DELETE /notifications/1 def destroy - @notification = Notification.find(params[:id]) @notification.destroy respond_to do |format| diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb index cfeda51a7..def0a018a 100644 --- a/app/controllers/orders_controller.rb +++ b/app/controllers/orders_controller.rb @@ -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| diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 07274b97b..056e7f3f5 100644 --- a/app/controllers/photos_controller.rb +++ b/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." diff --git a/app/controllers/plant_parts_controller.rb b/app/controllers/plant_parts_controller.rb index ed67926c7..e91bf57ba 100644 --- a/app/controllers/plant_parts_controller.rb +++ b/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| diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb index 708fc448d..00b080946 100644 --- a/app/controllers/plantings_controller.rb +++ b/app/controllers/plantings_controller.rb @@ -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") diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 76befd29d..2df88beab 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -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| diff --git a/app/controllers/products_controller.rb b/app/controllers/products_controller.rb index cb2f96e52..fa5108250 100644 --- a/app/controllers/products_controller.rb +++ b/app/controllers/products_controller.rb @@ -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| diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 91bcf2e52..5c8b6f03d 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -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| diff --git a/app/controllers/scientific_names_controller.rb b/app/controllers/scientific_names_controller.rb index 0fd0eccac..981dd5c9f 100644 --- a/app/controllers/scientific_names_controller.rb +++ b/app/controllers/scientific_names_controller.rb @@ -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 } @@ -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 diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb index 01fdc7beb..85f181e45 100644 --- a/app/controllers/seeds_controller.rb +++ b/app/controllers/seeds_controller.rb @@ -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 } @@ -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| From 3e43a19e58d4bb37707f99343fdd988e8bdec5f2 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:09:04 +1030 Subject: [PATCH 65/97] Update CONTRIBUTING.md Dead link, no archived version of it --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 71e94bd6b..1197b826c 100644 --- a/CONTRIBUTING.md +++ b/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). From 303fd8c24393c8d9f134967d62a0c5e488762bfb Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:20:09 +1030 Subject: [PATCH 66/97] Update autoprefixer-rails --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 425491475..31d0b316b 100644 --- a/Gemfile.lock +++ b/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) From 59ec36320b6e17cd626b7934a46099cf5c9f5b0c Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:20:47 +1030 Subject: [PATCH 67/97] Update codeclimate-test-reporter --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 31d0b316b..63a5bcbea 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) From aaaca81d496c5e7f9a28c8c2b969fb4f856f33cd Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:21:31 +1030 Subject: [PATCH 68/97] Update hashie --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 63a5bcbea..4672d6f9e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -216,7 +216,7 @@ GEM 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) From d99f24c02c25d9c0caa700a8cbb306cb9727d0f4 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:22:32 +1030 Subject: [PATCH 69/97] Update coffee-script-source --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 4672d6f9e..9034c1ed3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) From 18ab47eed37a3e0f6be03806bfb84a7062bd78f4 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:23:24 +1030 Subject: [PATCH 70/97] Update thor --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 9034c1ed3..23a548cf1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -461,7 +461,7 @@ 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) From 817c1ec5ce76f9867b30a3d682b78395fce835b5 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:24:18 +1030 Subject: [PATCH 71/97] Update will_paginate --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 23a548cf1..7d975ec4c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -486,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) From f7ca706e0bd4c460898c01ff84ba1f0cf33a86d8 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:25:08 +1030 Subject: [PATCH 72/97] Update omniauth-facebook (not as scary as it sounds - https://github.com/mkdynamic/omniauth-facebook/blob/master/CHANGELOG.md) --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7d975ec4c..c96a75e8b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -280,7 +280,7 @@ GEM mini_portile2 (2.1.0) minitest (5.9.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) @@ -299,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) From c9127dbf1e7247f3e733cf403970e295c8c9d890 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:46:22 +1030 Subject: [PATCH 73/97] Update rake and rubocop --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index c96a75e8b..00b5dd780 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -320,7 +320,7 @@ GEM cocaine (~> 0.5.5) mime-types mimemagic (~> 0.3.0) - parser (2.3.2.0) + parser (2.3.3.1) ast (~> 2.2) pg (0.19.0) plupload-rails (1.2.1) @@ -376,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) @@ -408,7 +408,7 @@ GEM rspec-mocks (~> 3.5.0) rspec-support (~> 3.5.0) rspec-support (3.5.0) - rubocop (0.45.0) + rubocop (0.46.0) parser (>= 2.3.1.1, < 3.0) powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) From 1982dc76be4fd5717103e05cb2afb712b444b852 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:51:13 +1030 Subject: [PATCH 74/97] Pin to elasticsearch API 2.0 for now --- Gemfile | 5 +++++ Gemfile.lock | 1 + 2 files changed, 6 insertions(+) diff --git a/Gemfile b/Gemfile index a91ccba45..6033dbb24 100644 --- a/Gemfile +++ b/Gemfile @@ -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" diff --git a/Gemfile.lock b/Gemfile.lock index 00b5dd780..32ccd887e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -518,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 From d3d3731fa319dee701568a354267d2e509efa424 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 10:52:01 +1030 Subject: [PATCH 75/97] Update minitest --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 32ccd887e..6d8716ff4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -278,7 +278,7 @@ 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.6.0) multipart-post (2.0.0) From 5235b11905f3fa1d94ded24fd353237ff36484f9 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 11:17:54 +1030 Subject: [PATCH 76/97] Disable Style/EmptyMethod for now --- .rubocop.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 43f27ddf4..06f48aefc 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -75,4 +75,7 @@ Style/FrozenStringLiteralComment: Rails/Output: Exclude: - 'config/unicorn.rb' - - 'db/seeds.rb' \ No newline at end of file + - 'db/seeds.rb' + +Style/EmptyMethod: + Enabled: false \ No newline at end of file From be572b9a539f22f54a864b6aed221465ae938b33 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Fri, 9 Dec 2016 11:25:03 +1030 Subject: [PATCH 77/97] Upgrade --- spec/spec_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7b314e901..ee8556548 100644 --- a/spec/spec_helper.rb +++ b/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 From 6ff3fa21e17b32f94fd6270731fc20ebf41b49c4 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Mon, 12 Dec 2016 10:37:26 +1030 Subject: [PATCH 78/97] Revert "Disable Style/EmptyMethod for now" This reverts commit 5235b11905f3fa1d94ded24fd353237ff36484f9. --- .rubocop.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 06f48aefc..43f27ddf4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -75,7 +75,4 @@ Style/FrozenStringLiteralComment: Rails/Output: Exclude: - 'config/unicorn.rb' - - 'db/seeds.rb' - -Style/EmptyMethod: - Enabled: false \ No newline at end of file + - 'db/seeds.rb' \ No newline at end of file From 74657abec053fa39841971e3ecc1ed96ec6823b0 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Mon, 12 Dec 2016 10:37:45 +1030 Subject: [PATCH 79/97] Revert "Update rake and rubocop" This reverts commit c9127dbf1e7247f3e733cf403970e295c8c9d890. --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6d8716ff4..4307429cf 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -320,7 +320,7 @@ GEM cocaine (~> 0.5.5) mime-types mimemagic (~> 0.3.0) - parser (2.3.3.1) + parser (2.3.2.0) ast (~> 2.2) pg (0.19.0) plupload-rails (1.2.1) @@ -376,7 +376,7 @@ GEM thor (>= 0.18.1, < 2.0) rainbow (2.1.0) raindrops (0.17.0) - rake (12.0.0) + rake (11.3.0) rb-fsevent (0.9.8) rb-inotify (0.9.7) ffi (>= 0.5.0) @@ -408,7 +408,7 @@ GEM rspec-mocks (~> 3.5.0) rspec-support (~> 3.5.0) rspec-support (3.5.0) - rubocop (0.46.0) + rubocop (0.45.0) parser (>= 2.3.1.1, < 3.0) powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) From feff2cdfd43b796b2356dbc9d5fe60bbdd27be40 Mon Sep 17 00:00:00 2001 From: Daniel O'Connor Date: Mon, 12 Dec 2016 10:38:31 +1030 Subject: [PATCH 80/97] Upgrade rake only --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 4307429cf..ae037027d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -376,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) From ca7868b79a9025a6216091cf2d72a40a5714a051 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 14 Dec 2016 21:28:35 +0000 Subject: [PATCH 81/97] Use guard clauses --- .rubocop_todo.yml | 25 --- app/controllers/members_controller.rb | 47 ++--- app/helpers/harvests_helper.rb | 30 ++-- app/mailers/notifier.rb | 5 +- app/models/ability.rb | 165 +++++++++--------- app/models/crop.rb | 64 +++---- app/models/garden.rb | 18 +- app/models/harvest.rb | 26 +-- app/models/member.rb | 7 +- app/models/notification.rb | 8 +- app/models/order.rb | 4 +- app/models/order_item.rb | 4 +- app/validators/approved_validator.rb | 4 +- .../initializers/comfortable_mexican_sofa.rb | 5 +- lib/geocodable.rb | 13 +- .../haml/filters/growstuff_markdown_spec.rb | 14 +- spec/support/elasticsearch_helpers.rb | 11 +- 17 files changed, 179 insertions(+), 271 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a554d3d45..03c4b3489 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -284,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: diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index d13a5e843..f62b25b75 100644 --- a/app/controllers/members_controller.rb +++ b/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 diff --git a/app/helpers/harvests_helper.rb b/app/helpers/harvests_helper.rb index 67f668804..0352a2b9c 100644 --- a/app/helpers/harvests_helper.rb +++ b/app/helpers/harvests_helper.rb @@ -3,26 +3,22 @@ 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 - pluralize(number_to_human(harvest.quantity, strip_insignificant_zeros: true), harvest.unit) - else - "#{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 + "#{number_to_human(harvest.quantity, strip_insignificant_zeros: true)} #{harvest.unit}" end end diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index e97fedcda..569400ce6 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -33,10 +33,7 @@ class Notifier < ActionMailer::Base 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) diff --git a/app/models/ability.rb b/app/models/ability.rb index 9085d7962..b96e6cf77 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -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 diff --git a/app/models/crop.rb b/app/models/crop.rb index 8d14f981c..48aa2bd6d 100644 --- a/app/models/crop.rb +++ b/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] @@ -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,9 +164,7 @@ 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 popular_plant_parts end @@ -261,30 +255,26 @@ class Crop < ActiveRecord::Base # rubocop:disable Metrics/ClassLength 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? - 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" - rejection_notes - else - reason_for_rejection - end + return rejection_notes if reason_for_rejection == "other" + reason_for_rejection end # Crop.search(string) @@ -362,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 @@ -373,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 diff --git a/app/models/garden.rb b/app/models/garden.rb index 8f4e73678..9771423f7 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -47,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,11 +78,11 @@ 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 diff --git a/app/models/harvest.rb b/app/models/harvest.rb index 3c5b3ac96..919dd6526 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -60,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 diff --git a/app/models/member.rb b/app/models/member.rb index 598507063..9940797ad 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -170,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 diff --git a/app/models/notification.rb b/app/models/notification.rb index f19de9b34..7061d7a18 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -17,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 diff --git a/app/models/order.rb b/app/models/order.rb index 81caa7f8e..9adb53fb0 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -56,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) diff --git a/app/models/order_item.rb b/app/models/order_item.rb index da26f3a42..ad85dc37c 100644 --- a/app/models/order_item.rb +++ b/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 diff --git a/app/validators/approved_validator.rb b/app/validators/approved_validator.rb index 4016058aa..e845b9529 100644 --- a/app/validators/approved_validator.rb +++ b/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 diff --git a/config/initializers/comfortable_mexican_sofa.rb b/config/initializers/comfortable_mexican_sofa.rb index 3206e14d9..d3dd4a326 100644 --- a/config/initializers/comfortable_mexican_sofa.rb +++ b/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 diff --git a/lib/geocodable.rb b/lib/geocodable.rb index 68b137acf..3ac7f3e8f 100644 --- a/lib/geocodable.rb +++ b/lib/geocodable.rb @@ -6,16 +6,13 @@ module Geocodable private def geocode - unless self.location.blank? - self.latitude, self.longitude = - Geocoder.coordinates(location, params: { limit: 1 }) - end + return if self.location.blank? + self.latitude, self.longitude = Geocoder.coordinates(location, params: { limit: 1 }) 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 diff --git a/spec/lib/haml/filters/growstuff_markdown_spec.rb b/spec/lib/haml/filters/growstuff_markdown_spec.rb index 3ee3102f4..d2fa77258 100644 --- a/spec/lib/haml/filters/growstuff_markdown_spec.rb +++ b/spec/lib/haml/filters/growstuff_markdown_spec.rb @@ -8,11 +8,8 @@ end def output_link(crop, name = nil) url = Rails.application.routes.url_helpers.crop_url(crop, host: Growstuff::Application.config.host) - if name - return "#{name}" - else - return "#{crop.name}" - end + return "#{name}" if name + "#{crop.name}" end def input_member_link(name) @@ -21,11 +18,8 @@ end def output_member_link(member, name = nil) url = Rails.application.routes.url_helpers.member_url(member, only_path: true) - if name - return "#{name}" - else - return "#{member.login_name}" - end + return "#{name}" if name + "#{member.login_name}" end describe 'Haml::Filters::Growstuff_Markdown' do diff --git a/spec/support/elasticsearch_helpers.rb b/spec/support/elasticsearch_helpers.rb index 1d1db1db3..1c564d661 100644 --- a/spec/support/elasticsearch_helpers.rb +++ b/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 From b0dd53eb63e88f030fe8e91e171a0e47f537e307 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sat, 17 Dec 2016 20:19:04 +1300 Subject: [PATCH 82/97] Upgrade to ruby 2.3.3 --- .ruby-version | 2 +- .travis.yml | 2 +- Gemfile | 2 +- Gemfile.lock | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.ruby-version b/.ruby-version index 2bf1c1ccf..0bee604df 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.3.1 +2.3.3 diff --git a/.travis.yml b/.travis.yml index 901cab8fe..ab1dcfed6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,7 @@ 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 - > diff --git a/Gemfile b/Gemfile index 6033dbb24..77ab2c4a6 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ # frozen_string_literal: true source 'https://rubygems.org' -ruby '2.3.1' +ruby '2.3.3' gem 'rails', '~> 4.2.7' diff --git a/Gemfile.lock b/Gemfile.lock index ae037027d..895448fad 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -570,7 +570,7 @@ DEPENDENCIES will_paginate (~> 3.0) RUBY VERSION - ruby 2.3.1p112 + ruby 2.3.3p222 BUNDLED WITH 1.13.6 From b40974d4c3b607d40f0315b2a96f54769a3a958f Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:15:19 +0000 Subject: [PATCH 83/97] Test Crop.create_from_csv with missing parent I couldn't work out how to test that the error is logged. --- spec/models/crop_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index cc962957c..438c6a56b 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -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" From b011fc9bae1230bd35d2312b4da4ce29eb164a6c Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:17:31 +0000 Subject: [PATCH 84/97] Test Post.comment_count --- spec/models/post_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 375b56586..bcb144a42 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -34,6 +34,13 @@ describe Post do @post.comments.size.should == 2 end + it "supports counting comments" do + @post = FactoryGirl.create(:post, author: member) + @comment1 = FactoryGirl.create(:comment, post: @post) + @comment2 = 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) From c23e80b56d4feaa6ff4bdd6bde831ea3a025d58b Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:26:17 +0000 Subject: [PATCH 85/97] Test Post.recently_active --- spec/models/post_spec.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index bcb144a42..18822016f 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -76,7 +76,7 @@ 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 @@ -90,14 +90,17 @@ describe Post do 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 From 7239cf068b36badb0a106ed683ab3ab894a22cc3 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:31:32 +0000 Subject: [PATCH 86/97] Test [member name](member) mention syntax --- spec/models/post_spec.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 18822016f..db7a4d295 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -107,12 +107,18 @@ describe Post do 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) }.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 From 59b86c9b0cfc30c02e14687fbbdcfaea92dce20e Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:33:41 +0000 Subject: [PATCH 87/97] Replace << with string-interpolation in post_spec --- spec/models/post_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index db7a4d295..eedfabc96 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -109,7 +109,7 @@ describe Post 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 @@ -120,7 +120,7 @@ describe Post do end it "sets the notification field" do - @p = FactoryGirl.create(:post, author: member, body: "Hey @" << member2.login_name) + @p = FactoryGirl.create(:post, author: member, body: "Hey @#{member2}") @n = Notification.first @n.sender.should eq member @n.recipient.should eq member2 @@ -129,15 +129,15 @@ describe Post do 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 From 67faa5554c12af195ff2408128fbcbd21a62f926 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:36:46 +0000 Subject: [PATCH 88/97] post_spec: replace instance vars with local vars The @ in front of a variable makes it an instance variable of the surrounding object. There's no point doing that for variables that are only used in a single function, so I've made such variables in the file post_spec.rb into function-local variables. We should do this more generally: there are a lot of instances of this (anti?)pattern in our test suite. --- spec/models/post_spec.rb | 70 ++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index eedfabc96..e73435162 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -18,57 +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) + comment1 = FactoryGirl.create(:comment, post: post) + comment2 = FactoryGirl.create(:comment, post: post) + post.comments.size.should == 2 end it "supports counting comments" do - @post = FactoryGirl.create(:post, author: member) - @comment1 = FactoryGirl.create(:comment, post: @post) - @comment2 = FactoryGirl.create(:comment, post: @post) - @post.comment_count.should == 2 + post = FactoryGirl.create(:post, author: member) + comment1 = FactoryGirl.create(:comment, post: post) + comment2 = 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) + comment1 = FactoryGirl.create(:comment, post: post) + comment2 = 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 @@ -83,9 +83,9 @@ describe Post do 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 @@ -120,12 +120,12 @@ describe Post do end it "sets the notification field" do - @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 + 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 From abb9acd04a0e73b8fa97a23d2a6ecde2ddc6059d Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 16:40:05 +0000 Subject: [PATCH 89/97] post_spec: fix warnings Several "unused variable" warnings (due to replacing unused instance variables with unused locals) and one warning about ambiguous lack-of-brackets. --- spec/models/post_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index e73435162..bed466d71 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -29,22 +29,22 @@ describe Post do it "has many comments" do post = FactoryGirl.create(:post, author: member) - comment1 = FactoryGirl.create(:comment, post: post) - comment2 = FactoryGirl.create(:comment, post: post) + 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) - comment1 = FactoryGirl.create(:comment, post: post) - comment2 = FactoryGirl.create(:comment, post: post) + 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) + FactoryGirl.create(:comment, post: post) + FactoryGirl.create(:comment, post: post) post.comments.size.should == 2 all = Comment.count post.destroy @@ -124,7 +124,7 @@ describe Post do n = Notification.first n.sender.should eq member n.recipient.should eq member2 - n.subject.should match /mentioned you in their post/ + n.subject.should match(/mentioned you in their post/) n.body.should eq p.body end From b8385afb2f8e5d3413fc510aeff0bacc3653e37d Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 20:14:05 +0000 Subject: [PATCH 90/97] Remove apparently-unused Geocodable.geocode method We have unit tests for geocoding, but it's not being covered, so I assume it's being shadowed by a method installed directly from Geocoder. --- lib/geocodable.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/geocodable.rb b/lib/geocodable.rb index 68b137acf..3d736bc20 100644 --- a/lib/geocodable.rb +++ b/lib/geocodable.rb @@ -5,13 +5,6 @@ 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 From 3469d6d37fe6ac1c40888443481377a5c688a6e9 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 20:23:07 +0000 Subject: [PATCH 91/97] Feature test for signin via email address --- spec/features/signin_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/features/signin_spec.rb b/spec/features/signin_spec.rb index 7bc08147c..d1eac9c42 100644 --- a/spec/features/signin_spec.rb +++ b/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' From f5eede60729832937378caaf1a8aaae583890735 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 20:29:05 +0000 Subject: [PATCH 92/97] Improve tests for Member.interesting --- spec/models/member_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index d5c9fb9b1..f2a4964c9 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -266,9 +266,11 @@ describe 'member' do @member1 = FactoryGirl.create(:london_member) @member2 = FactoryGirl.create(:london_member) @member3 = FactoryGirl.create(:london_member) - @member4 = FactoryGirl.create(:unconfirmed_member) + @member4 = FactoryGirl.create(:unconfirmed_member) # !1 + @member5 = FactoryGirl.create(:london_member) # 1, 2, !3 + @member6 = FactoryGirl.create(:member) # 1, !2, 3 - [@member1, @member2, @member3, @member4].each do |m| + [@member1, @member2, @member3, @member4, @member6].each do |m| FactoryGirl.create(:planting, owner: m) end From 70f589d59b3a97a64620cbdf5b964e9afa7daf4d Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 20:33:43 +0000 Subject: [PATCH 93/97] Replace instance vars with locals in member_spec --- spec/models/member_spec.rb | 126 ++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index f2a4964c9..736eb5844 100644 --- a/spec/models/member_spec.rb +++ b/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) + planting = 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) + comment1 = FactoryGirl.create(:comment, author: member) + comment2 = 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) + forum1 = FactoryGirl.create(:forum, owner: member) + forum2 = 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 @@ -198,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 @@ -221,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 @@ -229,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 @@ -263,38 +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) # !1 - @member5 = FactoryGirl.create(:london_member) # 1, 2, !3 - @member6 = FactoryGirl.create(:member) # 1, !2, 3 + member1 = FactoryGirl.create(:london_member) + member2 = FactoryGirl.create(:london_member) + member3 = FactoryGirl.create(:london_member) + member4 = FactoryGirl.create(:unconfirmed_member) # !1 + member5 = FactoryGirl.create(:london_member) # 1, 2, !3 + member6 = FactoryGirl.create(:member) # 1, !2, 3 - [@member1, @member2, @member3, @member4, @member6].each do |m| + [member1, member2, member3, member4, member6].each do |m| FactoryGirl.create(:planting, owner: m) end - @member1.updated_at = 3.days.ago - @member2.updated_at = 2.days.ago - @member3.updated_at = 1.day.ago + member1.updated_at = 3.days.ago + member2.updated_at = 2.days.ago + member3.updated_at = 1.day.ago - Member.interesting.should eq [@member3, @member2, @member1] + Member.interesting.should eq [member3, member2, member1] 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) + order1 = 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) + order1 = FactoryGirl.create(:completed_order, member: member) + order2 = FactoryGirl.create(:completed_order, member: member) + member.current_order.should be_nil end end @@ -302,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 From 4f1c94bfb93a5b9754d355d9d3eec64e65a04946 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 20:44:18 +0000 Subject: [PATCH 94/97] Fix warnings in member_spec --- spec/models/member_spec.rb | 40 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 736eb5844..19f3faa0b 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -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 @@ -263,37 +263,37 @@ 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) # !1 - member5 = FactoryGirl.create(:london_member) # 1, 2, !3 - member6 = FactoryGirl.create(:member) # 1, !2, 3 + 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, member6].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.day.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) + 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) + FactoryGirl.create(:completed_order, member: member) + FactoryGirl.create(:completed_order, member: member) member.current_order.should be_nil end end From 4b1cdc5650fa51d21432e4fb003db8b65c0877c2 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 19 Dec 2016 22:15:53 +0000 Subject: [PATCH 95/97] Test Model.newsletter_(un)subscribe I'm not very happy with this change as-is. The tests are very shallow (they wouldn't have caught the bug @tconquest and I found in this code, for instance), and use the deprecated `receive_message_chain` method. From the RSpec docs: "Chains can be arbitrarily long, which makes it quite painless to violate the Law of Demeter in violent ways, so you should consider any use of receive_message_chain a code smell. Even though not all code smells indicate real problems (think fluent interfaces), receive_message_chain still results in brittle examples. For example, if you write allow(foo).to receive_message_chain(:bar, :baz => 37) in a spec and then the implementation calls foo.baz.bar, the stub will not work." Further work needed. --- app/models/member.rb | 6 ++---- spec/models/member_spec.rb | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/models/member.rb b/app/models/member.rb index 598507063..a3260b663 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -239,9 +239,8 @@ 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 gb.lists.subscribe({ id: Growstuff::Application.config.newsletter_list_id, email: { email: email }, @@ -250,9 +249,8 @@ class Member < ActiveRecord::Base }) 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 gb.lists.unsubscribe({ id: Growstuff::Application.config.newsletter_list_id, email: { email: email } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 19f3faa0b..b23f0f882 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -401,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 From 3b1753d3d55f4809fde9bd189279a6be6f393ea9 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Mon, 2 Jan 2017 20:36:25 +1300 Subject: [PATCH 96/97] Garden's name in link --- app/helpers/gardens_helper.rb | 4 ++++ app/views/gardens/_thumbnail.html.haml | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/helpers/gardens_helper.rb b/app/helpers/gardens_helper.rb index 5d1895e8f..8e5b1ce60 100644 --- a/app/helpers/gardens_helper.rb +++ b/app/helpers/gardens_helper.rb @@ -9,6 +9,10 @@ module GardensHelper end end + def display_garden_name(garden) + truncate(garden.name, length: 50, separator: ' ', omission: '... ') + end + def display_garden_plantings(plantings) if plantings.blank? "None" diff --git a/app/views/gardens/_thumbnail.html.haml b/app/views/gardens/_thumbnail.html.haml index bc08be246..5bbfa1367 100644 --- a/app/views/gardens/_thumbnail.html.haml +++ b/app/views/gardens/_thumbnail.html.haml @@ -1,7 +1,7 @@ .panel.panel-success .panel-heading %h3.panel-title - = link_to "#{garden.owner.login_name}'s garden", garden + = link_to display_garden_name(garden), garden - if can? :edit, garden %a.pull-right{:href => edit_garden_path(garden), :role => "button", :id => "edit_garden_glyphicon"} %span.glyphicon.glyphicon-pencil{:title => "Edit"} @@ -12,7 +12,7 @@ .col-md-8 %dl.dl-horizontal %dt Name : - %dd= link_to garden.name, garden + %dd= link_to display_garden_name(garden), garden %dt Location : %dd - if garden.location.blank? From 881acdccc987260036c369ae1c445472a56b3123 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Mon, 2 Jan 2017 20:54:22 +1300 Subject: [PATCH 97/97] Shortening long garden names on a user's page --- app/views/members/_gardens.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/members/_gardens.html.haml b/app/views/members/_gardens.html.haml index 6dfeaa1c8..6050f6396 100644 --- a/app/views/members/_gardens.html.haml +++ b/app/views/members/_gardens.html.haml @@ -5,7 +5,7 @@ - member.gardens.each do |g| %li{:class => first_garden ? 'active' : '' } - first_garden = false - = link_to g.name, "#garden#{g.id}", 'data-toggle' => 'tab' + = link_to display_garden_name(g), "#garden#{g.id}", 'data-toggle' => 'tab' - if current_member == member %li.navbar-right = link_to new_garden_path, class: 'btn' do