From 36290c82ed6033507d1f557ad01be763affe46c1 Mon Sep 17 00:00:00 2001 From: Skud Date: Mon, 29 Apr 2013 18:09:55 +1000 Subject: [PATCH 01/19] Added a reply link to the notifications/show page - sets subject "Re: whatever" for PMs - also removed "sender=N" from PM link (wtf! -- it wasn't doing anything but it looked bad.) --- app/controllers/notifications_controller.rb | 16 ++++++++++++- app/views/members/show.html.haml | 2 +- app/views/notifications/_form.html.haml | 2 +- app/views/notifications/index.html.haml | 2 +- app/views/notifications/show.html.haml | 3 ++- .../notifications_controller_spec.rb | 24 ++++++++++++++++++- .../views/notifications/new.html.haml_spec.rb | 17 ++++++++++++- .../notifications/show.html.haml_spec.rb | 5 ++++ 8 files changed, 64 insertions(+), 7 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 7b49c326f..5fa2415df 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -15,6 +15,19 @@ class NotificationsController < ApplicationController @notification.read = true @notification.save + # by default, reply link sends a PM in return + @reply_link = new_notification_path( + :recipient_id => @notification.sender.id, + :subject => @notification.subject =~ /^Re: / ? + @notification.subject : + "Re: " + @notification.subject + ) + + if @notification.post + # comment on the post in question + @reply_link = new_comment_path(:post_id => @notification.post.id) + end + respond_to do |format| format.html # show.html.erb end @@ -25,6 +38,7 @@ class NotificationsController < ApplicationController def new @notification = Notification.new @recipient = Member.find_by_id(params[:recipient_id]) + @subject = params[:subject] || "" respond_to do |format| format.html # new.html.erb @@ -49,7 +63,7 @@ class NotificationsController < ApplicationController respond_to do |format| if @notification.save - format.html { redirect_to @recipient, notice: 'Message was successfully sent.' } + format.html { redirect_to notifications_path, notice: 'Message was successfully sent.' } else format.html { render action: "new" } end diff --git a/app/views/members/show.html.haml b/app/views/members/show.html.haml index 39a7893af..bd150618b 100644 --- a/app/views/members/show.html.haml +++ b/app/views/members/show.html.haml @@ -9,7 +9,7 @@ -if can? :create, Notification and current_member != @member %p %br/ - =link_to 'Send Message', new_notification_path(:sender_id => current_member.id, :recipient_id => @member.id), :class => 'btn btn-primary' + =link_to 'Send Message', new_notification_path(:recipient_id => @member.id), :class => 'btn btn-primary' %p %strong Member since: diff --git a/app/views/notifications/_form.html.haml b/app/views/notifications/_form.html.haml index a0b9f6d55..babb75fd0 100644 --- a/app/views/notifications/_form.html.haml +++ b/app/views/notifications/_form.html.haml @@ -13,7 +13,7 @@ To: = link_to @recipient, @recipient = label_tag :notification, "Subject:" - = f.text_field :subject, :class => 'input-block-level' + = f.text_field :subject, :value => @subject, :class => 'input-block-level' = label_tag :body, "Type your message here:" = f.text_area :body, :rows => 12, :class => 'input-block-level' %span.help-block diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 08c00121b..ace28ca92 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -1,4 +1,4 @@ -- content_for :title, "Notifications" +- content_for :title, "Inbox" - if @notifications.length > 0 %table.table.table-striped diff --git a/app/views/notifications/show.html.haml b/app/views/notifications/show.html.haml index cbd66aeaf..3d4280640 100644 --- a/app/views/notifications/show.html.haml +++ b/app/views/notifications/show.html.haml @@ -15,4 +15,5 @@ #{ strip_tags(@notification.body) } %p - =link_to 'Delete', @notification, method: :delete, data: { confirm: 'Are you sure?' }, :class => 'btn btn-mini' + =link_to 'Delete', @notification, method: :delete, data: { confirm: 'Are you sure?' }, :class => 'btn' + =link_to 'Reply', @reply_link, :class => 'btn btn-primary' diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index 155273f07..c6ca70b7e 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -42,6 +42,28 @@ describe NotificationsController do assigns(:notification).should eq(notification) end + it "assigns the reply link for a PM" do + notification = FactoryGirl.create(:notification, :recipient_id => subject.current_member.id, :post_id => nil) + subject = "Re: " + notification.subject + + get :show, {:id => notification.to_param} + assigns(:reply_link).should_not be_nil + assigns(:reply_link).should eq new_notification_path( + :recipient_id => notification.sender_id, + :subject => subject + ) + end + + it "assigns the reply link for a post comment" do + notification = FactoryGirl.create(:notification, :recipient_id => subject.current_member.id) + + get :show, {:id => notification.to_param} + assigns(:reply_link).should_not be_nil + assigns(:reply_link).should eq new_comment_path( + :post_id => notification.post.id + ) + end + it "marks notifications as read" do notification = FactoryGirl.create(:notification, :recipient_id => subject.current_member.id) get :show, {:id => notification.to_param} @@ -96,7 +118,7 @@ describe NotificationsController do it "redirects to the recipient's profile" do @recipient = FactoryGirl.create(:member) post :create, { :notification => { :recipient_id => @recipient.id } } - response.should redirect_to(@recipient) + response.should redirect_to(notifications_path) end end diff --git a/spec/views/notifications/new.html.haml_spec.rb b/spec/views/notifications/new.html.haml_spec.rb index 441905ef0..d96358da8 100644 --- a/spec/views/notifications/new.html.haml_spec.rb +++ b/spec/views/notifications/new.html.haml_spec.rb @@ -5,7 +5,6 @@ describe "notifications/new" do @recipient = FactoryGirl.create(:member) @sender = FactoryGirl.create(:member) assign(:notification, FactoryGirl.create(:notification, :recipient_id => @recipient.id, :sender_id => @sender.id)) -# assign(:forum, Forum.new) sign_in @sender controller.stub(:current_user) { @sender} end @@ -23,6 +22,22 @@ describe "notifications/new" do rendered.should contain @recipient.login_name end + it "puts the recipient in a hidden field" do + render + assert_select "input#notification_recipient_id[type=hidden]", :name => "notification[recipient_id]" + end + + it "fills in the subject if provided" do + assign(:subject, 'Foo') + render + assert_select "input#notification_subject", :value => "Foo" + end + + it "leaves the subject empty if not provided" do + render + assert_select "input#notification_subject", :value => "" + end + it "Tells you to write your message here" do render rendered.should contain "Type your message here" diff --git a/spec/views/notifications/show.html.haml_spec.rb b/spec/views/notifications/show.html.haml_spec.rb index 26251395b..8b9f2159c 100644 --- a/spec/views/notifications/show.html.haml_spec.rb +++ b/spec/views/notifications/show.html.haml_spec.rb @@ -5,6 +5,7 @@ describe "notifications/show" do @member = FactoryGirl.create(:member) @notification = FactoryGirl.create(:notification, :recipient => @member) assign(:notification, @notification) + assign(:reply_link, new_notification_path) controller.stub(:current_user) { @member } render end @@ -18,4 +19,8 @@ describe "notifications/show" do assert_select "a", "Delete" end + it "includes a reply button" do + assert_select "a", "Reply" + end + end From 844f2c845e963c9dca7804e2fa2a7eb181a59c65 Mon Sep 17 00:00:00 2001 From: Skud Date: Mon, 29 Apr 2013 18:26:22 +1000 Subject: [PATCH 02/19] removed unused random crop code --- app/helpers/application_helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 11d1c7f13..a2f487023 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,7 +1,3 @@ module ApplicationHelper - def random_crop - Crop.random - end - end From 11f979313a7e3d2b30d55012a7fa133b0c70576a Mon Sep 17 00:00:00 2001 From: Skud Date: Mon, 29 Apr 2013 18:55:13 +1000 Subject: [PATCH 03/19] Added reply link to notification email. Note: this breaks DRY but we couldn't figure out how to put it in a helper and make it useable by both the controller and the mailer. Suggestions/help much appreciated! --- app/mailers/notifier.rb | 14 ++++++++++++++ app/views/notifier/notify.html.haml | 2 ++ spec/views/notifier/notify.html.haml_spec.rb | 6 ++++++ 3 files changed, 22 insertions(+) diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 50612c353..28814bb07 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -3,6 +3,20 @@ class Notifier < ActionMailer::Base def notify(notification) @notification = notification + + # by default, reply link sends a PM in return + @reply_link = new_notification_url( + :recipient_id => @notification.sender.id, + :subject => @notification.subject =~ /^Re: / ? + @notification.subject : + "Re: " + @notification.subject + ) + + if @notification.post + # comment on the post in question + @reply_link = new_comment_url(:post_id => @notification.post.id) + end + mail(:to => @notification.recipient.email, :subject => @notification.subject) end diff --git a/app/views/notifier/notify.html.haml b/app/views/notifier/notify.html.haml index cbbb82bcd..7ccf9bea6 100644 --- a/app/views/notifier/notify.html.haml +++ b/app/views/notifier/notify.html.haml @@ -14,6 +14,8 @@ #{strip_tags @notification.body} %p + = link_to "Reply to this message", @reply_link + %br/ = link_to "View this message in your inbox", notification_url(@notification) %br/ = link_to "Turn off these notifications", edit_member_registration_url diff --git a/spec/views/notifier/notify.html.haml_spec.rb b/spec/views/notifier/notify.html.haml_spec.rb index 3f70895c1..687e702cc 100644 --- a/spec/views/notifier/notify.html.haml_spec.rb +++ b/spec/views/notifier/notify.html.haml_spec.rb @@ -4,6 +4,8 @@ describe 'notifier/notify.html.haml', :type => "view" do before(:each) do @notification = FactoryGirl.create(:notification) + @reply_link = "http://example.com" + assign(:reply_link, @reply_link) render end @@ -16,6 +18,10 @@ describe 'notifier/notify.html.haml', :type => "view" do rendered.should contain @notification.post.subject end + it 'should include a reply link' do + assert_select "a[href=#{@reply_link}]", :text => /Reply/ + end + it 'should contain a link to your inbox' do assert_select "a[href*=notifications]" end From 939a10d2f44c3b14920b0c4bedfca9fcd79bc1fb Mon Sep 17 00:00:00 2001 From: gnattery Date: Tue, 9 Apr 2013 21:18:01 +1000 Subject: [PATCH 04/19] Post subjects cannot be nil --- .../20130409103549_make_post_subject_non_null.rb | 3 +++ db/schema.rb | 10 +--------- spec/models/post_spec.rb | 5 +++++ 3 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20130409103549_make_post_subject_non_null.rb diff --git a/db/migrate/20130409103549_make_post_subject_non_null.rb b/db/migrate/20130409103549_make_post_subject_non_null.rb new file mode 100644 index 000000000..0a7f8db34 --- /dev/null +++ b/db/migrate/20130409103549_make_post_subject_non_null.rb @@ -0,0 +1,3 @@ +class MakePostSubjectNonNull < ActiveRecord::Migration + change_column :posts, :subject, :string, :null => false +end diff --git a/db/schema.rb b/db/schema.rb index 4c82164fe..1dd917e6b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130418102558) do +ActiveRecord::Schema.define(:version => 20130409162140) do create_table "authentications", :force => true do |t| t.integer "member_id", :null => false @@ -120,14 +120,6 @@ ActiveRecord::Schema.define(:version => 20130418102558) do t.datetime "updated_at", :null => false end - create_table "payments", :force => true do |t| - t.integer "payer_id" - t.string "payment_type" - t.decimal "amount" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false - end - create_table "plantings", :force => true do |t| t.integer "garden_id", :null => false t.integer "crop_id", :null => false diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 450a31893..473a514df 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -43,6 +43,11 @@ describe Post do @post.forum.should be_an_instance_of Forum end + it "doesn't allow a blank subject" do + @post = FactoryGirl.build(:post, :subject => nil) + expect { @post.save }.to raise_error ActiveRecord::StatementInvalid + end + context "recent activity" do before(:each) do Time.stub(:now => Time.now) From 9d4caaa2b51b6c695804816f413995283d6a3022 Mon Sep 17 00:00:00 2001 From: gnattery Date: Tue, 9 Apr 2013 21:43:38 +1000 Subject: [PATCH 05/19] Do validation for empty and spaces-only strings --- app/models/post.rb | 5 +++++ spec/models/post_spec.rb | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 28152ca56..6622969c7 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -10,6 +10,11 @@ class Post < ActiveRecord::Base default_scope order("created_at desc") + validates :subject, + :format => { + :with => /\S/ + } + def author_date_subject # slugs are created before created_at is set time = created_at || Time.zone.now diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb index 473a514df..8e7f91083 100644 --- a/spec/models/post_spec.rb +++ b/spec/models/post_spec.rb @@ -43,9 +43,19 @@ describe Post do @post.forum.should be_an_instance_of Forum end - it "doesn't allow a blank subject" do + it "doesn't allow a nil subject" do @post = FactoryGirl.build(:post, :subject => nil) - expect { @post.save }.to raise_error ActiveRecord::StatementInvalid + @post.should_not be_valid + end + + it "doesn't allow a blank subject" do + @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 end context "recent activity" do From aab0a438fd2db192da0645355e8c31da4fda104e Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Tue, 30 Apr 2013 12:39:03 +0100 Subject: [PATCH 06/19] Replace blank/nil notification subjects with (no subject) --- app/models/notification.rb | 7 +++ spec/models/notification_spec.rb | 10 ++++ .../notifications/index.html.haml_spec.rb | 46 +++++++++++++++---- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index 1be9da637..3fc48a0c7 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -9,12 +9,19 @@ class Notification < ActiveRecord::Base default_scope order('created_at DESC') scope :unread, where(:read => false) + before_create :replace_blank_subject after_create :send_email def self.unread_count self.unread.count end + def replace_blank_subject + if self.subject.nil? or self.subject =~ /^\s*$/ + self.subject = "(no subject)" + end + end + def send_email if self.recipient.send_notification_email Notifier.notify(self).deliver diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 8ebaba010..a8bddb9fa 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -48,4 +48,14 @@ describe Notification do ActionMailer::Base.deliveries.last.to.should == [@notification2.recipient.email] end + it "replaces missing subjects with (no subject)" do + @notification = FactoryGirl.create(:notification, :subject => nil) + @notification.subject.should == "(no subject)" + end + + it "replaces whitespace-only subjects with (no subject)" do + @notification = FactoryGirl.create(:notification, :subject => " ") + @notification.subject.should == "(no subject)" + end + end diff --git a/spec/views/notifications/index.html.haml_spec.rb b/spec/views/notifications/index.html.haml_spec.rb index 0e65dc93f..351bbb44f 100644 --- a/spec/views/notifications/index.html.haml_spec.rb +++ b/spec/views/notifications/index.html.haml_spec.rb @@ -4,19 +4,45 @@ describe "notifications/index" do before(:each) do @member = FactoryGirl.create(:member) controller.stub(:current_user) { @member } - @notification = FactoryGirl.create(:notification, :sender => @member, :recipient => @member) - assign(:notifications, [ @notification, @notification ]) - render end - it "renders a list of notifications" do - # Run the generator again with the --webrat flag if you want to use webrat matchers - assert_select "table" - assert_select "tr>td", :text => @notification.sender.to_s, :count => 2 - assert_select "tr>td", :text => @notification.subject, :count => 2 + context "ordinary notifications" do + before(:each) do + @notification = FactoryGirl.create(:notification, :sender => @member, + :recipient => @member) + assign(:notifications, [ @notification, @notification ]) + render + end + + it "renders a list of notifications" do + assert_select "table" + assert_select "tr>td", :text => @notification.sender.to_s, :count => 2 + assert_select "tr>td", :text => @notification.subject, :count => 2 + end + + it "links to sender's profile" do + assert_select "a", :href => member_path(@notification.sender) + end end - it "links to sender's profile" do - assert_select "a", :href => member_path(@notification.sender) + context "no subject" do + it "shows (no subject)" do + @notification = FactoryGirl.create(:notification, + :sender => @member, :recipient => @member, :subject => nil) + assign(:notifications, [@notification]) + render + rendered.should contain "(no subject)" + end end + + context "whitespace-only subject" do + it "shows (no subject)" do + @notification = FactoryGirl.create(:notification, + :sender => @member, :recipient => @member, :subject => " ") + assign(:notifications, [@notification]) + render + rendered.should contain "(no subject)" + end + end + end From e0782c23460bc157c5d9ae6be35354aea52b856e Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 30 Apr 2013 21:56:02 +1000 Subject: [PATCH 07/19] fixed failing notifications tests (our non-DRY code, which we just pasted into the mailer, was breaking this in a roundabout way. this is a quick and not very nice fix. The whole problem is resolved by @pozorvlak's recent "no subject" fix for notifications, anyway.) --- spec/controllers/notifications_controller_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index c6ca70b7e..635ccb38b 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -7,7 +7,8 @@ describe NotificationsController do def valid_attributes { "recipient_id" => subject.current_member.id, - "sender_id" => FactoryGirl.create(:member).id + "sender_id" => FactoryGirl.create(:member).id, + "subject" => 'test' } end @@ -19,7 +20,8 @@ describe NotificationsController do def valid_attributes_for_sender { "sender_id" => subject.current_member.id, - "recipient_id" => FactoryGirl.create(:member).id + "recipient_id" => FactoryGirl.create(:member).id, + "subject" => 'test' } end @@ -117,7 +119,7 @@ describe NotificationsController do it "redirects to the recipient's profile" do @recipient = FactoryGirl.create(:member) - post :create, { :notification => { :recipient_id => @recipient.id } } + post :create, { :notification => { :recipient_id => @recipient.id, :subject => 'foo' } } response.should redirect_to(notifications_path) end end From c46ba2f16ba0389866cc3e683bafdd29ca653417 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Wed, 1 May 2013 16:27:29 +0100 Subject: [PATCH 08/19] Delete unused helpers. --- app/helpers/application_helper.rb | 3 --- app/helpers/comments_helper.rb | 2 -- app/helpers/crops_helper.rb | 2 -- app/helpers/forums_helper.rb | 2 -- app/helpers/gardens_helper.rb | 2 -- app/helpers/home_helper.rb | 2 -- app/helpers/members_helper.rb | 9 --------- app/helpers/notifications_helper.rb | 2 -- app/helpers/plantings_helper.rb | 2 -- app/helpers/posts_helper.rb | 2 -- app/helpers/roles_helper.rb | 2 -- app/helpers/scientific_names_helper.rb | 2 -- spec/helpers/plantings_helper_spec.rb | 14 -------------- 13 files changed, 46 deletions(-) delete mode 100644 app/helpers/application_helper.rb delete mode 100644 app/helpers/comments_helper.rb delete mode 100644 app/helpers/crops_helper.rb delete mode 100644 app/helpers/forums_helper.rb delete mode 100644 app/helpers/gardens_helper.rb delete mode 100644 app/helpers/home_helper.rb delete mode 100644 app/helpers/members_helper.rb delete mode 100644 app/helpers/notifications_helper.rb delete mode 100644 app/helpers/plantings_helper.rb delete mode 100644 app/helpers/posts_helper.rb delete mode 100644 app/helpers/roles_helper.rb delete mode 100644 app/helpers/scientific_names_helper.rb delete mode 100644 spec/helpers/plantings_helper_spec.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb deleted file mode 100644 index a2f487023..000000000 --- a/app/helpers/application_helper.rb +++ /dev/null @@ -1,3 +0,0 @@ -module ApplicationHelper - -end diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb deleted file mode 100644 index 0ec9ca5f2..000000000 --- a/app/helpers/comments_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module CommentsHelper -end diff --git a/app/helpers/crops_helper.rb b/app/helpers/crops_helper.rb deleted file mode 100644 index da6fb9a78..000000000 --- a/app/helpers/crops_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module CropsHelper -end diff --git a/app/helpers/forums_helper.rb b/app/helpers/forums_helper.rb deleted file mode 100644 index 2e531fd46..000000000 --- a/app/helpers/forums_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module ForumsHelper -end diff --git a/app/helpers/gardens_helper.rb b/app/helpers/gardens_helper.rb deleted file mode 100644 index 015e7835f..000000000 --- a/app/helpers/gardens_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module GardensHelper -end diff --git a/app/helpers/home_helper.rb b/app/helpers/home_helper.rb deleted file mode 100644 index 23de56ac6..000000000 --- a/app/helpers/home_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module HomeHelper -end diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb deleted file mode 100644 index 51a8599e2..000000000 --- a/app/helpers/members_helper.rb +++ /dev/null @@ -1,9 +0,0 @@ -module MembersHelper - def shorten(str, max_length) - if str.length > max_length - return str.slice(0, max_length - 3) + "..." - else - return str - end - end -end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb deleted file mode 100644 index 7342393a7..000000000 --- a/app/helpers/notifications_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module NotificationsHelper -end diff --git a/app/helpers/plantings_helper.rb b/app/helpers/plantings_helper.rb deleted file mode 100644 index bc609d05e..000000000 --- a/app/helpers/plantings_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module PlantingsHelper -end diff --git a/app/helpers/posts_helper.rb b/app/helpers/posts_helper.rb deleted file mode 100644 index a7b8cec89..000000000 --- a/app/helpers/posts_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module PostsHelper -end diff --git a/app/helpers/roles_helper.rb b/app/helpers/roles_helper.rb deleted file mode 100644 index 6b6b6cc22..000000000 --- a/app/helpers/roles_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module RolesHelper -end diff --git a/app/helpers/scientific_names_helper.rb b/app/helpers/scientific_names_helper.rb deleted file mode 100644 index 3b1a12371..000000000 --- a/app/helpers/scientific_names_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module ScientificNamesHelper -end diff --git a/spec/helpers/plantings_helper_spec.rb b/spec/helpers/plantings_helper_spec.rb deleted file mode 100644 index 25a84ec96..000000000 --- a/spec/helpers/plantings_helper_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' - -# Specs in this file have access to a helper object that includes -# the PlantingsHelper. For example: -# -# describe PlantingsHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# helper.concat_strings("this","that").should == "this that" -# end -# end -# end -describe PlantingsHelper do -end From 6ab098c015e4302b7d5e5fb8356ec14e683f9f2a Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Wed, 1 May 2013 16:46:54 +0100 Subject: [PATCH 09/19] Move reply_link into a helper. - keep @reply_link as an instance var, for testability - always use fully-qualified URLs, to reduce code duplication --- app/controllers/notifications_controller.rb | 15 ++------ app/helpers/notifications_helper.rb | 16 +++++++++ app/mailers/notifier.rb | 15 ++------ .../notifications_controller_spec.rb | 4 +-- spec/helpers/notifications_helper_spec.rb | 34 +++++++++++++++++++ .../notifications/show.html.haml_spec.rb | 4 +-- 6 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 app/helpers/notifications_helper.rb create mode 100644 spec/helpers/notifications_helper_spec.rb diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 5fa2415df..790e5d3ac 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,4 +1,5 @@ class NotificationsController < ApplicationController + include NotificationsHelper load_and_authorize_resource # GET /notifications def index @@ -14,19 +15,7 @@ class NotificationsController < ApplicationController @notification = Notification.find(params[:id]) @notification.read = true @notification.save - - # by default, reply link sends a PM in return - @reply_link = new_notification_path( - :recipient_id => @notification.sender.id, - :subject => @notification.subject =~ /^Re: / ? - @notification.subject : - "Re: " + @notification.subject - ) - - if @notification.post - # comment on the post in question - @reply_link = new_comment_path(:post_id => @notification.post.id) - end + @reply_link = reply_link(@notification) respond_to do |format| format.html # show.html.erb diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb new file mode 100644 index 000000000..dab53df95 --- /dev/null +++ b/app/helpers/notifications_helper.rb @@ -0,0 +1,16 @@ +module NotificationsHelper + def reply_link(notification) + if notification.post + # comment on the post in question + new_comment_url(:post_id => notification.post.id) + else + # by default, reply link sends a PM in return + new_notification_url( + :recipient_id => notification.sender.id, + :subject => notification.subject =~ /^Re: / ? + notification.subject : + "Re: " + notification.subject + ) + end + end +end diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 28814bb07..579b2a8ad 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -1,21 +1,10 @@ class Notifier < ActionMailer::Base + include NotificationsHelper default from: "Growstuff " def notify(notification) @notification = notification - - # by default, reply link sends a PM in return - @reply_link = new_notification_url( - :recipient_id => @notification.sender.id, - :subject => @notification.subject =~ /^Re: / ? - @notification.subject : - "Re: " + @notification.subject - ) - - if @notification.post - # comment on the post in question - @reply_link = new_comment_url(:post_id => @notification.post.id) - end + @reply_link = reply_link(@notification) mail(:to => @notification.recipient.email, :subject => @notification.subject) diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index 635ccb38b..be3fe5400 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -50,7 +50,7 @@ describe NotificationsController do get :show, {:id => notification.to_param} assigns(:reply_link).should_not be_nil - assigns(:reply_link).should eq new_notification_path( + assigns(:reply_link).should eq new_notification_url( :recipient_id => notification.sender_id, :subject => subject ) @@ -61,7 +61,7 @@ describe NotificationsController do get :show, {:id => notification.to_param} assigns(:reply_link).should_not be_nil - assigns(:reply_link).should eq new_comment_path( + assigns(:reply_link).should eq new_comment_url( :post_id => notification.post.id ) end diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb new file mode 100644 index 000000000..9db3a95aa --- /dev/null +++ b/spec/helpers/notifications_helper_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe NotificationsHelper do + describe "reply_link" do + + before(:each) do + @member = FactoryGirl.create(:member) + end + + 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 new_notification_url( + :recipient_id => notification.sender_id, + :subject => subject + ) + end + + it "replies to post comments with post comments" do + notification = FactoryGirl.create(:notification, :recipient_id => @member.id) + + link = helper.reply_link(notification) + link.should_not be_nil + link.should eq new_comment_url( + :post_id => notification.post.id + ) + end + + + end +end diff --git a/spec/views/notifications/show.html.haml_spec.rb b/spec/views/notifications/show.html.haml_spec.rb index 8b9f2159c..3028e8f0e 100644 --- a/spec/views/notifications/show.html.haml_spec.rb +++ b/spec/views/notifications/show.html.haml_spec.rb @@ -5,7 +5,7 @@ describe "notifications/show" do @member = FactoryGirl.create(:member) @notification = FactoryGirl.create(:notification, :recipient => @member) assign(:notification, @notification) - assign(:reply_link, new_notification_path) + @reply_link = assign(:reply_link, new_notification_path) controller.stub(:current_user) { @member } render end @@ -20,7 +20,7 @@ describe "notifications/show" do end it "includes a reply button" do - assert_select "a", "Reply" + assert_select "a[href=#{@reply_link}]", "Reply" end end From 4d372ab8f8588eface93c5b79a481896f4d552fc Mon Sep 17 00:00:00 2001 From: Lilly Date: Thu, 2 May 2013 12:13:28 +1000 Subject: [PATCH 10/19] Added crop wrangling hints to crops form. --- app/views/crops/_form.html.haml | 28 +++++++++++++++++--------- spec/views/crops/new.html.haml_spec.rb | 5 ++++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/app/views/crops/_form.html.haml b/app/views/crops/_form.html.haml index 12c917702..a225c1a92 100644 --- a/app/views/crops/_form.html.haml +++ b/app/views/crops/_form.html.haml @@ -1,4 +1,4 @@ -= form_for @crop do |f| += form_for @crop, :html => {:class => 'form-horizontal'} do |f| - if @crop.errors.any? #error_explanation %h3= "#{pluralize(@crop.errors.count, "error")} prohibited this crop from being saved:" @@ -6,11 +6,21 @@ - @crop.errors.full_messages.each do |msg| %li= msg - .field - = f.label :system_name - = f.text_field :system_name - .field - = f.label :en_wikipedia_url - = f.text_field :en_wikipedia_url - .actions - = f.submit 'Save' + %p + %span.help-block + For detailed crop wrangling guidelines, please consult the + =link_to "crop wrangling guide", "http://wiki.growstuff.org/index.php/Crop_wrangling" + on the Growstuff wiki. + + .control-group + = f.label :system_name, :class => 'control-label' + .controls + = f.text_field :system_name + %span.help-inline Name in US English; singular; capitalize proper nouns only. + .control-group + = f.label :en_wikipedia_url, 'Wikipedia URL', :class => 'control-label' + .controls + = f.text_field :en_wikipedia_url + %span.help-inline Link to this crop's page on the English language Wikipedia. + .form-actions + = f.submit 'Save', :class => 'btn btn-primary' diff --git a/spec/views/crops/new.html.haml_spec.rb b/spec/views/crops/new.html.haml_spec.rb index 548e0acd3..4016a27a3 100644 --- a/spec/views/crops/new.html.haml_spec.rb +++ b/spec/views/crops/new.html.haml_spec.rb @@ -10,7 +10,6 @@ describe "crops/new" do end it "renders new crop form" do - render # Run the generator again with the --webrat flag if you want to use webrat matchers assert_select "form", :action => crops_path, :method => "post" do assert_select "input#crop_system_name", :name => "crop[system_name]" @@ -18,4 +17,8 @@ describe "crops/new" do end end + it "shows a link to crop wrangling guidelines" do + assert_select "a[href^=http://wiki.growstuff.org]", "crop wrangling guide" + end + end From e5526a32925cfa1daaa74436c4c0cb17291c1272 Mon Sep 17 00:00:00 2001 From: Skud Date: Thu, 2 May 2013 13:27:12 +1000 Subject: [PATCH 11/19] added validation for en_wikipedia_url --- app/models/crop.rb | 6 ++++++ spec/models/crop_spec.rb | 20 ++++++++------------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/app/models/crop.rb b/app/models/crop.rb index 7190e048b..99db0166c 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -6,6 +6,12 @@ class Crop < ActiveRecord::Base has_many :plantings default_scope order("lower(system_name) asc") + validates :en_wikipedia_url, + :format => { + :with => /^https?:\/\/en\.wikipedia\.org\/wiki/, + :message => 'is not a valid English Wikipedia URL' + } + def Crop.random @crop = Crop.offset(rand(Crop.count)).first return @crop diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index 8e336d2ba..aca3d2681 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -27,22 +27,11 @@ describe Crop do context 'invalid data' do it 'should not save a crop without a system name' do - @crop = Crop.new + @crop = FactoryGirl.build(:crop, :system_name => nil) expect { @crop.save }.to raise_error ActiveRecord::StatementInvalid end end - context 'random' do - before(:each) do - @crop = FactoryGirl.create(:tomato) - end - - it 'should find a random crop' do - @rand_crop = Crop.random - @rand_crop.system_name.should == 'Tomato' - end - end - context 'ordering' do it "should be sorted case-insensitively" do uppercase = FactoryGirl.create(:uppercasecrop) @@ -64,4 +53,11 @@ describe Crop do FactoryGirl.create(:planting, :crop => @c) @c.plantings_count.should eq 1 end + + it 'validates en_wikipedia_url' do + @crop = FactoryGirl.build(:tomato, :en_wikipedia_url => 'this is not valid') + @crop.should_not be_valid + @crop = FactoryGirl.build(:tomato, :en_wikipedia_url => 'http://en.wikipedia.org/wiki/SomePage') + @crop.should be_valid + end end From ad8a0110c53febe046521eee51176cf6d1825591 Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 3 May 2013 12:35:02 +1000 Subject: [PATCH 12/19] One-off rake task to clean up empty notification subjects Also fixed admin_user task to use login_name (case sensitive) rather than the id-like stub generated by friendlyid (always lowercase). --- lib/tasks/growstuff.rake | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/tasks/growstuff.rake b/lib/tasks/growstuff.rake index 784410df0..64140750a 100644 --- a/lib/tasks/growstuff.rake +++ b/lib/tasks/growstuff.rake @@ -5,9 +5,27 @@ namespace :growstuff do task :admin_user => :environment do - member = Member.find(ENV['name']) or raise "Usage: rake growstuff:admin_user name=whoever" + member = Member.find_by_login_name(ENV['name']) + or raise "Usage: rake growstuff:admin_user name=whoever (login name is case-sensitive)" admin = Role.find_or_create_by_name!('admin') member.roles << admin + end + + + namespace :oneoff do + desc "One-off tasks needed at various times and kept for posterity" + + task :empty_subjects => :environment do + desc "May 2013: replace any empty notification subjects with (no subject)" + + # this is inefficient as it checks every Notification, but the + # site is small and there aren't many of them, so it shouldn't matter + # for this one-off script. + Notification.all.each do |n| + n.replace_blank_subject + n.save + end + end end From d4e5f0e32aeb3b090ee6377d88ac32e6f88fea72 Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 8 May 2013 20:23:55 +1000 Subject: [PATCH 13/19] fixed linebreak causing syntax error --- db/schema.rb | 30 +++++++++++++++++++++++++++++- lib/tasks/growstuff.rake | 3 +-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 1dd917e6b..090be1b27 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130409162140) do +ActiveRecord::Schema.define(:version => 20130508050711) do create_table "authentications", :force => true do |t| t.integer "member_id", :null => false @@ -120,6 +120,26 @@ ActiveRecord::Schema.define(:version => 20130409162140) do t.datetime "updated_at", :null => false end + create_table "orders", :force => true do |t| + t.string "member_id", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + t.datetime "completed_at" + end + + create_table "orders_products", :id => false, :force => true do |t| + t.integer "order_id" + t.integer "product_id" + end + + create_table "payments", :force => true do |t| + t.integer "payer_id" + t.string "payment_type" + t.decimal "amount" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + end + create_table "plantings", :force => true do |t| t.integer "garden_id", :null => false t.integer "crop_id", :null => false @@ -147,6 +167,14 @@ ActiveRecord::Schema.define(:version => 20130409162140) do add_index "posts", ["created_at", "author_id"], :name => "index_updates_on_created_at_and_user_id" add_index "posts", ["slug"], :name => "index_updates_on_slug", :unique => true + create_table "products", :force => true do |t| + t.string "name", :null => false + t.string "description", :null => false + t.decimal "min_price", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + end + create_table "roles", :force => true do |t| t.string "name", :null => false t.text "description" diff --git a/lib/tasks/growstuff.rake b/lib/tasks/growstuff.rake index 64140750a..e75887bd8 100644 --- a/lib/tasks/growstuff.rake +++ b/lib/tasks/growstuff.rake @@ -5,8 +5,7 @@ namespace :growstuff do task :admin_user => :environment do - member = Member.find_by_login_name(ENV['name']) - or raise "Usage: rake growstuff:admin_user name=whoever (login name is case-sensitive)" + member = Member.find_by_login_name(ENV['name']) or raise "Usage: rake growstuff:admin_user name=whoever (login name is case-sensitive)" admin = Role.find_or_create_by_name!('admin') member.roles << admin end From 1348a2a492f1b0cd5e43b58c83cb21764ee7a563 Mon Sep 17 00:00:00 2001 From: Ryan Clark Date: Wed, 8 May 2013 13:13:14 -0700 Subject: [PATCH 14/19] Add validation for name in garden model with test. --- app/models/garden.rb | 2 ++ spec/models/garden_spec.rb | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/app/models/garden.rb b/app/models/garden.rb index d8ed4056d..83ce39283 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -7,6 +7,8 @@ class Garden < ActiveRecord::Base has_many :plantings, :order => 'created_at DESC', :dependent => :destroy has_many :crops, :through => :plantings + validates_presence_of :name + default_scope order("lower(name) asc") def garden_slug diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb index 1523c504b..1def495d2 100644 --- a/spec/models/garden_spec.rb +++ b/spec/models/garden_spec.rb @@ -14,6 +14,12 @@ describe Garden do @garden.description.should == "This is a **totally** cool garden" end + it "should have a name" do + @no_name_garden = FactoryGirl.build(:garden, :name => nil, :description => "New Garden") + @no_name_garden.should_not be_valid + @no_name_garden.errors[:name].should include("can't be blank") + end + it "should have an owner" do @garden.owner.should be_an_instance_of Member end From 536cf21e3edf67d1beb1182fea61911addd43e27 Mon Sep 17 00:00:00 2001 From: Skud Date: Thu, 9 May 2013 11:39:22 +1000 Subject: [PATCH 15/19] make it easier to wrangle scientific names --- .../scientific_names_controller.rb | 8 ++-- app/views/crops/show.html.haml | 31 ++++++++++----- app/views/scientific_names/_form.html.haml | 26 ++++++++----- app/views/scientific_names/edit.html.haml | 2 +- app/views/scientific_names/new.html.haml | 2 +- spec/controllers/crops_controller_spec.rb | 5 ++- .../scientific_names_controller_spec.rb | 39 +++++++++++++++++-- spec/factories/scientific_name.rb | 5 +++ spec/views/crops/show.html.haml_spec.rb | 10 ++++- .../scientific_names/edit.html.haml_spec.rb | 2 +- .../scientific_names/new.html.haml_spec.rb | 2 +- 11 files changed, 100 insertions(+), 32 deletions(-) diff --git a/app/controllers/scientific_names_controller.rb b/app/controllers/scientific_names_controller.rb index b54cac6af..691c51872 100644 --- a/app/controllers/scientific_names_controller.rb +++ b/app/controllers/scientific_names_controller.rb @@ -26,6 +26,7 @@ class ScientificNamesController < ApplicationController # GET /scientific_names/new.json def new @scientific_name = ScientificName.new + @crop = Crop.find_by_id(params[:crop_id]) || Crop.new respond_to do |format| format.html # new.html.haml @@ -45,7 +46,7 @@ class ScientificNamesController < ApplicationController respond_to do |format| if @scientific_name.save - format.html { redirect_to @scientific_name, notice: 'Scientific name was successfully created.' } + format.html { redirect_to @scientific_name.crop, notice: 'Scientific name was successfully created.' } format.json { render json: @scientific_name, status: :created, location: @scientific_name } else format.html { render action: "new" } @@ -61,7 +62,7 @@ class ScientificNamesController < ApplicationController respond_to do |format| if @scientific_name.update_attributes(params[:scientific_name]) - format.html { redirect_to @scientific_name, notice: 'Scientific name was successfully updated.' } + format.html { redirect_to @scientific_name.crop, notice: 'Scientific name was successfully updated.' } format.json { head :no_content } else format.html { render action: "edit" } @@ -74,10 +75,11 @@ class ScientificNamesController < ApplicationController # DELETE /scientific_names/1.json def destroy @scientific_name = ScientificName.find(params[:id]) + @crop = @scientific_name.crop @scientific_name.destroy respond_to do |format| - format.html { redirect_to scientific_names_url } + format.html { redirect_to @crop } format.json { head :no_content } end end diff --git a/app/views/crops/show.html.haml b/app/views/crops/show.html.haml index e3364fa15..30612955c 100644 --- a/app/views/crops/show.html.haml +++ b/app/views/crops/show.html.haml @@ -9,19 +9,30 @@ = render :partial => "plantings/thumbnail", :locals => { :planting => p, :title => 'owner' } .span3 + - if can? :edit, @crop or can? :destroy, @crop + %h4 Crop wrangling + %p + You are a + = succeed "." do + %strong CROP WRANGLER + %p + - if can? :edit, @crop + = link_to 'Edit crop', edit_crop_path(@crop), { :class => 'btn btn-mini' } + - if can? :destroy, @crop + = link_to 'Delete crop', @crop, method: :delete, data: { confirm: 'Are you sure?' }, :class => 'btn btn-mini' + %h4 Scientific names: %ul - @crop.scientific_names.each do |sn| - %li= sn.scientific_name + %li + = sn.scientific_name + - if can? :edit, sn + = link_to 'Edit', edit_scientific_name_path(sn), { :class => 'btn btn-mini' } + - if can? :destroy, sn + = link_to 'Delete', sn, method: :delete, data: { confirm: 'Are you sure?' }, :class => 'btn btn-mini' + %p + - if can? :edit, @crop + = link_to 'Add', new_scientific_name_path( :crop_id => @crop.id ), { :class => 'btn btn-mini' } %h4 More information: %ul %li= link_to 'Wikipedia (English)', @crop.en_wikipedia_url - - - if can? :edit, @crop or can? :destroy, @crop - %h4 Crop wrangling: - %p - - if can? :edit, @crop - = link_to 'Edit', edit_crop_path(@crop), { :class => 'btn btn-mini' } - - if can? :destroy, @crop - = link_to 'Delete', @crop, method: :delete, data: { confirm: 'Are you sure?' }, :class => 'btn btn-mini' - diff --git a/app/views/scientific_names/_form.html.haml b/app/views/scientific_names/_form.html.haml index d2389e58d..503d324a2 100644 --- a/app/views/scientific_names/_form.html.haml +++ b/app/views/scientific_names/_form.html.haml @@ -1,4 +1,4 @@ -= form_for @scientific_name do |f| += form_for @scientific_name, :html => {:class => 'form-horizontal'} do |f| - if @scientific_name.errors.any? #error_explanation %h2= "#{pluralize(@scientific_name.errors.count, "error")} prohibited this scientific_name from being saved:" @@ -6,11 +6,19 @@ - @scientific_name.errors.full_messages.each do |msg| %li= msg - .field - = f.label :scientific_name - = f.text_field :scientific_name - .field - = f.label :crop_id - = f.number_field :crop_id - .actions - = f.submit 'Save' + %p + %span.help-block + For detailed crop wrangling guidelines, please consult the + =link_to "crop wrangling guide", "http://wiki.growstuff.org/index.php/Crop_wrangling" + on the Growstuff wiki. + + .control-group + = f.label :crop_id, :class => 'control-label' + .controls + = collection_select(:scientific_name, :crop_id, Crop.all, :id, :system_name, :selected => @scientific_name.crop_id || @crop.id) + .control-group + = f.label :scientific_name, :class => 'control-label' + .controls + = f.text_field :scientific_name + .form-actions + = f.submit 'Save', :class => 'btn btn-primary' diff --git a/app/views/scientific_names/edit.html.haml b/app/views/scientific_names/edit.html.haml index 58d434c3e..493ee3e44 100644 --- a/app/views/scientific_names/edit.html.haml +++ b/app/views/scientific_names/edit.html.haml @@ -1,3 +1,3 @@ -%h1 Editing scientific_name +- content_for :title, "Edit scientific name" = render 'form' diff --git a/app/views/scientific_names/new.html.haml b/app/views/scientific_names/new.html.haml index af8bef174..104a4b5a6 100644 --- a/app/views/scientific_names/new.html.haml +++ b/app/views/scientific_names/new.html.haml @@ -1,3 +1,3 @@ -%h1 New scientific_name +- content_for :title, "New scientific name" = render 'form' diff --git a/spec/controllers/crops_controller_spec.rb b/spec/controllers/crops_controller_spec.rb index ed7b0e0e9..db4c070bc 100644 --- a/spec/controllers/crops_controller_spec.rb +++ b/spec/controllers/crops_controller_spec.rb @@ -5,7 +5,10 @@ describe CropsController do login_member(:crop_wrangling_member) def valid_attributes - { :system_name => "Tomato" } + { + :system_name => "Tomato", + :en_wikipedia_url => 'http://en.wikipedia.org/wiki/Tomato' + } end describe "GET index" do diff --git a/spec/controllers/scientific_names_controller_spec.rb b/spec/controllers/scientific_names_controller_spec.rb index dda252d6b..0cf431a4a 100644 --- a/spec/controllers/scientific_names_controller_spec.rb +++ b/spec/controllers/scientific_names_controller_spec.rb @@ -4,8 +4,12 @@ describe ScientificNamesController do login_member(:crop_wrangling_member) + before(:each) do + @crop = FactoryGirl.create(:tomato) + end + def valid_attributes - { :scientific_name => 'Solanum lycopersicum', :crop_id => 1 } + { :scientific_name => 'Solanum lycopersicum', :crop_id => @crop.id } end describe "GET index" do @@ -29,6 +33,32 @@ describe ScientificNamesController do get :new, {} assigns(:scientific_name).should be_a_new(ScientificName) end + + it "assigns crop if specified" do + get :new, { :crop_id => 1 } + assigns(:crop).should be_an_instance_of Crop + end + + it "assigns crop if specified" do + end + end + + describe "GET edit" do + it "assigns the requested scientific_name as @scientific_name" do + scientific_name = ScientificName.create! valid_attributes + get :edit, {:id => scientific_name.to_param} + assigns(:scientific_name).should eq(scientific_name) + end + end + + describe "POST create" do + describe "with valid params" do + it "creates a new ScientificName" do + expect { + post :create, {:scientific_name => valid_attributes} + }.to change(ScientificName, :count).by(1) + end + end end describe "GET edit" do @@ -55,7 +85,7 @@ describe ScientificNamesController do it "redirects to the created scientific_name" do post :create, {:scientific_name => valid_attributes} - response.should redirect_to(ScientificName.last) + response.should redirect_to(ScientificName.last.crop) end end @@ -97,7 +127,7 @@ describe ScientificNamesController do it "redirects to the scientific_name" do scientific_name = ScientificName.create! valid_attributes put :update, {:id => scientific_name.to_param, :scientific_name => valid_attributes} - response.should redirect_to(scientific_name) + response.should redirect_to(scientific_name.crop) end end @@ -130,8 +160,9 @@ describe ScientificNamesController do it "redirects to the scientific_names list" do scientific_name = ScientificName.create! valid_attributes + crop = scientific_name.crop delete :destroy, {:id => scientific_name.to_param} - response.should redirect_to(scientific_names_url) + response.should redirect_to(crop) end end diff --git a/spec/factories/scientific_name.rb b/spec/factories/scientific_name.rb index 7bb3e4cd9..e1bffb5cd 100644 --- a/spec/factories/scientific_name.rb +++ b/spec/factories/scientific_name.rb @@ -1,12 +1,17 @@ FactoryGirl.define do factory :scientific_name do + association :crop, factory: :crop + scientific_name "Beanus Magicus" + factory :zea_mays do association :crop, factory: :maize scientific_name "Zea mays" end + factory :solanum_lycopersicum do association :crop, factory: :tomato scientific_name "Solanum lycopersicum" end + end end diff --git a/spec/views/crops/show.html.haml_spec.rb b/spec/views/crops/show.html.haml_spec.rb index 8467e19b9..81099aabd 100644 --- a/spec/views/crops/show.html.haml_spec.rb +++ b/spec/views/crops/show.html.haml_spec.rb @@ -54,7 +54,15 @@ describe "crops/show" do end it "links to the edit crop form" do - rendered.should contain "Edit" + assert_select "a[href=#{edit_crop_path(@crop)}]", :text => "Edit crop" + end + + it "links to the add scientific name form" do + assert_select "a[href^=#{new_scientific_name_path}]", :text => "Add" + end + + it "links to the edit scientific name form" do + assert_select "a[href=#{edit_scientific_name_path(@crop.scientific_names.first)}]", :text => "Edit" end end diff --git a/spec/views/scientific_names/edit.html.haml_spec.rb b/spec/views/scientific_names/edit.html.haml_spec.rb index bdaa4b6c1..30e195f52 100644 --- a/spec/views/scientific_names/edit.html.haml_spec.rb +++ b/spec/views/scientific_names/edit.html.haml_spec.rb @@ -15,7 +15,7 @@ describe "scientific_names/edit" do it "renders the edit scientific_name form" do assert_select "form", :action => scientific_names_path(@scientific_name), :method => "post" do assert_select "input#scientific_name_scientific_name", :name => "scientific_name[scientific_name]" - assert_select "input#scientific_name_crop_id", :name => "scientific_name[crop_id]" + assert_select "select#scientific_name_crop_id", :name => "scientific_name[crop_id]" end end end diff --git a/spec/views/scientific_names/new.html.haml_spec.rb b/spec/views/scientific_names/new.html.haml_spec.rb index 9d4fc93ae..9c892f5ef 100644 --- a/spec/views/scientific_names/new.html.haml_spec.rb +++ b/spec/views/scientific_names/new.html.haml_spec.rb @@ -18,7 +18,7 @@ describe "scientific_names/new" do # Run the generator again with the --webrat flag if you want to use webrat matchers assert_select "form", :action => scientific_names_path, :method => "post" do assert_select "input#scientific_name_scientific_name", :name => "scientific_name[scientific_name]" - assert_select "input#scientific_name_crop_id", :name => "scientific_name[crop_id]" + assert_select "select#scientific_name_crop_id", :name => "scientific_name[crop_id]" end end From 14021a4fae787ce944c00b51cc2666d288e52286 Mon Sep 17 00:00:00 2001 From: Lilly Date: Thu, 9 May 2013 13:39:31 +1000 Subject: [PATCH 16/19] Redirect to Garden after deleting planting --- app/controllers/plantings_controller.rb | 3 ++- db/schema.rb | 22 +------------------ spec/controllers/plantings_controller_spec.rb | 5 +++-- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/app/controllers/plantings_controller.rb b/app/controllers/plantings_controller.rb index 98f038870..ff9ebe886 100644 --- a/app/controllers/plantings_controller.rb +++ b/app/controllers/plantings_controller.rb @@ -85,10 +85,11 @@ class PlantingsController < ApplicationController # DELETE /plantings/1.json def destroy @planting = Planting.find(params[:id]) + @garden = @planting.garden @planting.destroy respond_to do |format| - format.html { redirect_to plantings_url } + format.html { redirect_to @garden } format.json { head :no_content } end end diff --git a/db/schema.rb b/db/schema.rb index 090be1b27..4c82164fe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130508050711) do +ActiveRecord::Schema.define(:version => 20130418102558) do create_table "authentications", :force => true do |t| t.integer "member_id", :null => false @@ -120,18 +120,6 @@ ActiveRecord::Schema.define(:version => 20130508050711) do t.datetime "updated_at", :null => false end - create_table "orders", :force => true do |t| - t.string "member_id", :null => false - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false - t.datetime "completed_at" - end - - create_table "orders_products", :id => false, :force => true do |t| - t.integer "order_id" - t.integer "product_id" - end - create_table "payments", :force => true do |t| t.integer "payer_id" t.string "payment_type" @@ -167,14 +155,6 @@ ActiveRecord::Schema.define(:version => 20130508050711) do add_index "posts", ["created_at", "author_id"], :name => "index_updates_on_created_at_and_user_id" add_index "posts", ["slug"], :name => "index_updates_on_slug", :unique => true - create_table "products", :force => true do |t| - t.string "name", :null => false - t.string "description", :null => false - t.decimal "min_price", :null => false - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false - end - create_table "roles", :force => true do |t| t.string "name", :null => false t.text "description" diff --git a/spec/controllers/plantings_controller_spec.rb b/spec/controllers/plantings_controller_spec.rb index 77f9c519a..ff3fc9e36 100644 --- a/spec/controllers/plantings_controller_spec.rb +++ b/spec/controllers/plantings_controller_spec.rb @@ -161,10 +161,11 @@ describe PlantingsController do }.to change(Planting, :count).by(-1) end - it "redirects to the plantings list" do + it "redirects to the garden" do planting = Planting.create! valid_attributes + garden = planting.garden delete :destroy, {:id => planting.to_param} - response.should redirect_to(plantings_url) + response.should redirect_to(garden) end end From ee136666d5f816ea100186957e2f5fa0cd7c555b Mon Sep 17 00:00:00 2001 From: Ryan Clark Date: Thu, 9 May 2013 13:06:29 -0700 Subject: [PATCH 17/19] Replace name validation with before_filter and add rake tast for empty names. Replace validation testing with before_filter tests. --- Pathogen: | 1 + app/models/garden.rb | 8 +++++++- lib/tasks/growstuff.rake | 12 ++++++++++++ spec/models/garden_spec.rb | 15 ++++++++++++--- 4 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 Pathogen: diff --git a/Pathogen: b/Pathogen: new file mode 100644 index 000000000..a5b3293af --- /dev/null +++ b/Pathogen: @@ -0,0 +1 @@ +clean up extra line in garden_spec. diff --git a/app/models/garden.rb b/app/models/garden.rb index 83ce39283..029502a5a 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -7,7 +7,7 @@ class Garden < ActiveRecord::Base has_many :plantings, :order => 'created_at DESC', :dependent => :destroy has_many :crops, :through => :plantings - validates_presence_of :name + before_create :replace_blank_name default_scope order("lower(name) asc") @@ -31,6 +31,12 @@ class Garden < ActiveRecord::Base return unique_plantings[0..3] end + def replace_blank_name + if self.name.nil? or self.name =~ /^\s*$/ + self.name = "(no name)" + end + end + def to_s name end diff --git a/lib/tasks/growstuff.rake b/lib/tasks/growstuff.rake index e75887bd8..15cdb61a6 100644 --- a/lib/tasks/growstuff.rake +++ b/lib/tasks/growstuff.rake @@ -26,6 +26,18 @@ namespace :growstuff do end end + task :empty_names => :environment do + desc "May 2013: replace any empty garden names with (no subject)" + + # this is inefficient as it checks every Garden, but the + # site is small and there aren't many of them, so it shouldn't matter + # for this one-off script. + Garden.all.each do |n| + n.replace_blank_name + n.save + end + end + end end diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb index 1def495d2..d4d36636b 100644 --- a/spec/models/garden_spec.rb +++ b/spec/models/garden_spec.rb @@ -15,9 +15,8 @@ describe Garden do end it "should have a name" do - @no_name_garden = FactoryGirl.build(:garden, :name => nil, :description => "New Garden") - @no_name_garden.should_not be_valid - @no_name_garden.errors[:name].should include("can't be blank") + @no_name_garden = FactoryGirl.create(:garden, :name => nil, :description => "New Garden") + @no_name_garden.name.should_not be_blank end it "should have an owner" do @@ -87,4 +86,14 @@ describe Garden do Planting.count.should == all - 2 end + it "replaces missing name with (no name)" do + @no_name_garden = FactoryGirl.create(:garden, :name => nil) + @no_name_garden.name.should == "(no name)" + end + + it "replaces whitespace-only names with (no name)" do + @no_name_garden = FactoryGirl.create(:garden, :name => " ") + @no_name_garden.name.should == "(no name)" + end + end From 5d03ffdf87a3df8bd70fa4eaf923b2537090f014 Mon Sep 17 00:00:00 2001 From: Ryan Clark Date: Mon, 13 May 2013 17:28:37 -0700 Subject: [PATCH 18/19] Added validation for garden names. --- app/models/garden.rb | 7 ++++++- spec/models/garden_spec.rb | 26 +++++++++++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/app/models/garden.rb b/app/models/garden.rb index 029502a5a..a2bd553e7 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -7,10 +7,15 @@ class Garden < ActiveRecord::Base has_many :plantings, :order => 'created_at DESC', :dependent => :destroy has_many :crops, :through => :plantings - before_create :replace_blank_name + # before_create :replace_blank_name default_scope order("lower(name) asc") + validates :name, + :format => { + :with => /\S/ + } + def garden_slug "#{owner.login_name}-#{name}".downcase.gsub(' ', '-') end diff --git a/spec/models/garden_spec.rb b/spec/models/garden_spec.rb index d4d36636b..cbbf3a950 100644 --- a/spec/models/garden_spec.rb +++ b/spec/models/garden_spec.rb @@ -14,9 +14,19 @@ describe Garden do @garden.description.should == "This is a **totally** cool garden" end - it "should have a name" do - @no_name_garden = FactoryGirl.create(:garden, :name => nil, :description => "New Garden") - @no_name_garden.name.should_not be_blank + it "doesn't allow a nil name" do + @garden = FactoryGirl.build(:garden, :name => nil) + @garden.should_not be_valid + end + + it "doesn't allow a blank name" do + @garden = FactoryGirl.build(:garden, :name => "") + @garden.should_not be_valid + end + + it "doesn't allow a name with only spaces" do + @garden = FactoryGirl.build(:garden, :name => " ") + @garden.should_not be_valid end it "should have an owner" do @@ -86,14 +96,4 @@ describe Garden do Planting.count.should == all - 2 end - it "replaces missing name with (no name)" do - @no_name_garden = FactoryGirl.create(:garden, :name => nil) - @no_name_garden.name.should == "(no name)" - end - - it "replaces whitespace-only names with (no name)" do - @no_name_garden = FactoryGirl.create(:garden, :name => " ") - @no_name_garden.name.should == "(no name)" - end - end From 95a2007683c2bf53ee035862b4ab7314261e1993 Mon Sep 17 00:00:00 2001 From: Ryan Clark Date: Mon, 13 May 2013 17:36:37 -0700 Subject: [PATCH 19/19] fix up existing gardens with blank names. --- .gitignore | 1 + Pathogen: | 1 - app/models/garden.rb | 6 ------ lib/tasks/growstuff.rake | 12 +++++++----- 4 files changed, 8 insertions(+), 12 deletions(-) delete mode 100644 Pathogen: diff --git a/.gitignore b/.gitignore index ead788319..5b8787714 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ *~ *.DS_Store credentials.sh +Pathogen: \ No newline at end of file diff --git a/Pathogen: b/Pathogen: deleted file mode 100644 index a5b3293af..000000000 --- a/Pathogen: +++ /dev/null @@ -1 +0,0 @@ -clean up extra line in garden_spec. diff --git a/app/models/garden.rb b/app/models/garden.rb index a2bd553e7..13d8927c6 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -36,12 +36,6 @@ class Garden < ActiveRecord::Base return unique_plantings[0..3] end - def replace_blank_name - if self.name.nil? or self.name =~ /^\s*$/ - self.name = "(no name)" - end - end - def to_s name end diff --git a/lib/tasks/growstuff.rake b/lib/tasks/growstuff.rake index 15cdb61a6..fb87ea319 100644 --- a/lib/tasks/growstuff.rake +++ b/lib/tasks/growstuff.rake @@ -26,15 +26,17 @@ namespace :growstuff do end end - task :empty_names => :environment do - desc "May 2013: replace any empty garden names with (no subject)" + task :empty_garden_names => :environment do + desc "May 2013: replace any empty garden names with Garden" # this is inefficient as it checks every Garden, but the # site is small and there aren't many of them, so it shouldn't matter # for this one-off script. - Garden.all.each do |n| - n.replace_blank_name - n.save + Garden.all.each do |g| + if g.name.nil? or g.name =~ /^\s*$/ + g.name = "Garden" + g.save + end end end