From ac7b48406bd2c156c58043735a86e7cdd77eacd1 Mon Sep 17 00:00:00 2001 From: gnattery Date: Mon, 25 Mar 2013 17:52:50 +1100 Subject: [PATCH 01/23] Members can send each other private messages. Problems: Notifications controller test fails on redirecting, but seems to work fine in practice Send Message button doesn't look nice on page --- app/controllers/notifications_controller.rb | 31 ++++++++++ app/models/ability.rb | 5 +- app/models/notification.rb | 2 +- app/views/members/show.html.haml | 4 ++ app/views/notifications/_form.html.haml | 23 ++++++++ app/views/notifications/new.html.haml | 4 ++ config/routes.rb | 2 + .../notifications_controller_spec.rb | 59 +++++++++++++++++++ spec/factories/notifications.rb | 2 +- spec/views/members/show.html.haml_spec.rb | 12 ++++ .../views/notifications/new.html.haml_spec.rb | 36 +++++++++++ 11 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 app/views/notifications/_form.html.haml create mode 100644 app/views/notifications/new.html.haml create mode 100644 spec/views/notifications/new.html.haml_spec.rb diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 0c37bf28c..6cc9c13df 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -24,6 +24,20 @@ class NotificationsController < ApplicationController end end + # GET /notifications/new + # GET /notifications/new.json + + def new + @notification = Notification.new + @recipient = Member.find_by_id(params[:recipient_id]) + @sender = Member.find_by_id(params[:sender_id]) + + respond_to do |format| + format.html # new.html.erb + format.json { render json: @notification } + end + end + # DELETE /notifications/1 # DELETE /notifications/1.json def destroy @@ -35,4 +49,21 @@ class NotificationsController < ApplicationController format.json { head :no_content } end end + + # POST /notifications + # POST /notifications.json + def create + @notification = Notification.new(params[:notification]) + @recipient = Member.find_by_id(params[:notification][:recipient_id]) + + respond_to do |format| + if @notification.save + format.html { redirect_to @recipient, notice: 'Message was successfully sent.' } + format.json { render json: @notification, status: :created, location: @notification } + else + format.html { render action: "new" } + format.json { render json: @notification.errors, status: :unprocessable_entity } + end + end + end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 31e6b7b0e..b508f4f8d 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -7,6 +7,7 @@ class Ability # everyone can do these things, even non-logged in can :read, :all cannot :read, Notification + cannot :create, Notification # nobody should be able to view this except admins cannot :read, Role @@ -28,7 +29,9 @@ class Ability # can read/delete notifications that were sent to them can :read, Notification, :recipient_id => member.id can :destroy, Notification, :recipient_id => member.id - # note we don't support create/update for notifications + # can create notifications that were sent by them + can :create, Notification, :sender_id => member.id + # note we don't support update for notifications # only crop wranglers can create/edit/destroy crops if member.has_role? :crop_wrangler diff --git a/app/models/notification.rb b/app/models/notification.rb index 789f773ca..080a69b39 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,5 +1,5 @@ class Notification < ActiveRecord::Base - attr_accessible :sender_id, :recipient_id, + attr_accessible :sender_id, :recipient_id, :subject, :body, :post_id, :read belongs_to :sender, :class_name => 'Member' diff --git a/app/views/members/show.html.haml b/app/views/members/show.html.haml index 956c12e10..6e7e65e11 100644 --- a/app/views/members/show.html.haml +++ b/app/views/members/show.html.haml @@ -6,6 +6,10 @@ .span3 = render :partial => "members/avatar", :locals => { :member => @member } + -if current_member && current_member != @member + %p + =link_to 'Send Message', new_notification_path(:sender_id => current_member.id, :recipient_id => @member.id), :class => 'btn btn-primary' + %p = "Member since: #{@member.created_at.to_s(:date)}" - if @member.location.to_s != '' diff --git a/app/views/notifications/_form.html.haml b/app/views/notifications/_form.html.haml new file mode 100644 index 000000000..d00d6848b --- /dev/null +++ b/app/views/notifications/_form.html.haml @@ -0,0 +1,23 @@ += form_for @notification do |f| + - if @notification.errors.any? + #error_explanation + %h2= "#{pluralize(@post.errors.count, "error")} prohibited this message from being sent:" + %ul + - @notification.errors.full_messages.each do |msg| + %li= msg + + .field + = f.hidden_field :recipient_id, :value => @recipient.id + = f.hidden_field :sender_id, :value => @sender.id + + %p + = label_tag "You are sending a private message to: " + = link_to @recipient, @recipient + = label_tag :notification, "Subject" + = f.text_field :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 + = render :partial => "shared/markdown_help" + + = f.submit "Send", :class => 'btn btn-primary' diff --git a/app/views/notifications/new.html.haml b/app/views/notifications/new.html.haml new file mode 100644 index 000000000..87f89de3c --- /dev/null +++ b/app/views/notifications/new.html.haml @@ -0,0 +1,4 @@ += content_for :title, "Send a message" + +=render 'form' + diff --git a/config/routes.rb b/config/routes.rb index 413be78f9..eddf2d230 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,7 @@ Growstuff::Application.routes.draw do + get "notifications/new" + devise_for :members, :controllers => { :registrations => "registrations" } resources :plantings diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index 939ddb443..a1642b36f 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -36,6 +36,26 @@ describe NotificationsController do end end + describe "GET new" do + it "assigns a new notification as @notification" do + get :new, {} + assigns(:notification).should be_a_new(Notification) + end + + it "assigns a recipient" do + @recipient = FactoryGirl.create(:member) + get :new, {:recipient_id => @recipient.id } + assigns(:recipient).should be_an_instance_of(Member) + end + + it "assigns a sender" do + @sender = FactoryGirl.create(:member) + @notification = FactoryGirl.create(:notification, :sender => @sender) + get :new, {:sender_id => @sender.id } + assigns(:sender).should be_an_instance_of(Member) + end + end + describe "DELETE destroy" do it "destroys the requested notification" do notification = Notification.create! valid_attributes @@ -51,4 +71,43 @@ describe NotificationsController do end end + describe "POST create" do + describe "with valid params" do + it "creates a new notification" do + expect { + post :create, {:notification => valid_attributes} + }.to change(Notification, :count).by(1) + end + + it "assigns a newly created notification as @notification" do + post :create, {:notification => valid_attributes} + assigns(:notification).should be_a(Notification) + assigns(:notification).should be_persisted + end + + it "redirects to the recipient's profile" do + @sender = FactoryGirl.create(:member) + @recipient = FactoryGirl.create(:member) + post :create, { :notification => { :recipient_id => @recipient.id, + :sender_id => @sender.id}} + response.should redirect_to(@recipient) + end + end + + describe "with invalid params" do + it "assigns a newly created but unsaved notification as @notification" do + # Trigger the behavior that occurs when invalid params are submitted + Notification.any_instance.stub(:save).and_return(false) + post :create, {:notification => {}} + assigns(:notification).should be_a_new(Notification) + end + + it "re-renders the 'new' template" do + # Trigger the behavior that occurs when invalid params are submitted + Notification.any_instance.stub(:save).and_return(false) + post :create, {:notification => {}} + response.should render_template("new") + end + end + end end diff --git a/spec/factories/notifications.rb b/spec/factories/notifications.rb index aa26b4145..bd59adb8d 100644 --- a/spec/factories/notifications.rb +++ b/spec/factories/notifications.rb @@ -1,7 +1,7 @@ # Read about factories at https://github.com/thoughtbot/factory_girl FactoryGirl.define do - factory :notification do + factory :notification, aliases: [:message] do sender recipient subject "MyString" diff --git a/spec/views/members/show.html.haml_spec.rb b/spec/views/members/show.html.haml_spec.rb index 701972107..f141694ec 100644 --- a/spec/views/members/show.html.haml_spec.rb +++ b/spec/views/members/show.html.haml_spec.rb @@ -76,6 +76,10 @@ describe "members/show" do it "contains an edit settings button" do rendered.should contain "Edit Settings" end + + it "contains no send message button" do + rendered.should_not contain "Send Message" + end end context "signed in as different member" do @@ -97,6 +101,10 @@ describe "members/show" do it "contains no edit settings button" do rendered.should_not contain "Edit Settings" end + + it "contains a send message button" do + rendered.should contain "Send Message" + end end context "public member" do @@ -108,6 +116,10 @@ describe "members/show" do it "shows the email address" do rendered.should contain @member.email end + + it "doesn't show a send message button" do + rendered.should_not contain "Send Message" + end end context "geolocations" do diff --git a/spec/views/notifications/new.html.haml_spec.rb b/spec/views/notifications/new.html.haml_spec.rb new file mode 100644 index 000000000..441905ef0 --- /dev/null +++ b/spec/views/notifications/new.html.haml_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe "notifications/new" do + before(:each) 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 + + it "renders new message form" do + render + assert_select "form", :action => notifications_path, :method => "notification" do + assert_select "input#notification_subject", :name => "notification[subject]" + assert_select "textarea#notification_body", :name => "notification[body]" + end + end + + it "tells you who the recipient is" do + render + rendered.should contain @recipient.login_name + end + + it "Tells you to write your message here" do + render + rendered.should contain "Type your message here" + end + + it 'shows markdown help' do + render + rendered.should contain 'Markdown' + end + +end From 98eabe097bc4a2ef8ffaac5e1196f4607e69cb72 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Mon, 25 Mar 2013 13:32:26 +0000 Subject: [PATCH 02/23] Fix test failure on redirect The "create" method was never being called. Removing the sender_id parameter fixed that. Routing problem, maybe? --- spec/controllers/notifications_controller_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index a1642b36f..549f5a7f4 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -88,8 +88,7 @@ describe NotificationsController do it "redirects to the recipient's profile" do @sender = FactoryGirl.create(:member) @recipient = FactoryGirl.create(:member) - post :create, { :notification => { :recipient_id => @recipient.id, - :sender_id => @sender.id}} + post :create, { :notification => { :recipient_id => @recipient.id } } response.should redirect_to(@recipient) end end From b7b0948b04b1d08a150c019fdd9561b36bcd90a2 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 28 Mar 2013 10:18:53 +0000 Subject: [PATCH 03/23] Tidy up URL handlers in notification email. As per @jcaudle's comment https://github.com/Growstuff/growstuff/pull/152#issuecomment-15563523 --- app/views/notifier/notify.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/notifier/notify.html.haml b/app/views/notifier/notify.html.haml index 7623d2be8..80ad294cf 100644 --- a/app/views/notifier/notify.html.haml +++ b/app/views/notifier/notify.html.haml @@ -3,18 +3,18 @@ %p You have received a message from - = link_to @notification.sender.login_name, url_for(:controller => 'members', :action => 'show', :id => @notification.sender.id, :only_path => false ) + = link_to @notification.sender.login_name, member_url(@notification.sender) on #{site_name} at #{@notification.created_at.to_s(:date)} - if @notification.post in response to - = link_to @notification.post.subject, url_for(:controller => 'posts', :action => 'show', :id => @notification.post.id, :only_path => false) + = link_to @notification.post.subject, post_url(@notification.post) \. %blockquote :markdown #{strip_tags @notification.body} %p - = link_to "View this message in your inbox", url_for(:controller => 'notifications', :action => 'show', :id => @notification.id, :only_path => false) + = link_to "View this message in your inbox", notification_url(@notification) %br/ = link_to "Turn off these notifications", edit_member_registration_url From bb1e8ede199010297be7b0d14132f4cee21705a6 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 28 Mar 2013 10:43:47 +0000 Subject: [PATCH 04/23] Fix misordered comments on post page bug. https://www.pivotaltracker.com/story/show/46799417 --- app/models/comment.rb | 2 ++ spec/views/posts/show.html.haml_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/app/models/comment.rb b/app/models/comment.rb index 395d99277..ea2082bf9 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -3,6 +3,8 @@ class Comment < ActiveRecord::Base belongs_to :author, :class_name => 'Member' belongs_to :post + default_scope order("created_at asc") + after_create do recipient = self.post.author.id sender = self.author.id diff --git a/spec/views/posts/show.html.haml_spec.rb b/spec/views/posts/show.html.haml_spec.rb index aadf31a29..ee983c702 100644 --- a/spec/views/posts/show.html.haml_spec.rb +++ b/spec/views/posts/show.html.haml_spec.rb @@ -62,6 +62,25 @@ describe "posts/show" do end end + context "when there is more than one comment" do + before(:each) do + @post = assign(:post, + FactoryGirl.create(:html_post, :author => @author)) + @comment1 = FactoryGirl.create(:comment, :post => @post, :body => "F1rst!!!", + :created_at => Date.new(2010, 5, 17)) + @comment3 = FactoryGirl.create(:comment, :post => @post, :body => "Th1rd!!!", + :created_at => Date.new(2012, 5, 17)) + @comment4 = FactoryGirl.create(:comment, :post => @post, :body => "F0urth!!!") + @comment2 = FactoryGirl.create(:comment, :post => @post, :body => "S3c0nd!!1!", + :created_at => Date.new(2011, 5, 17)) + render + end + + it "shows the oldest comments first" do + rendered.should contain /#{@comment1.body}.*#{@comment2.body}.*#{@comment3.body}.*#{@comment4.body}/m + end + end + context "forum post" do it "shows forum name" do @post = assign(:post, From c9cc7be0d1ae67dc51acd35f4589298717787f25 Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Thu, 28 Mar 2013 10:57:30 +0000 Subject: [PATCH 05/23] Refactor model code out of view in posts/show. --- app/controllers/posts_controller.rb | 1 + app/views/posts/show.html.haml | 6 +++--- spec/views/posts/show.html.haml_spec.rb | 2 ++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index e725be734..1ca2b5f94 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -18,6 +18,7 @@ class PostsController < ApplicationController # GET /posts/1.json def show @post = Post.find(params[:id]) + @comments = @post.comments respond_to do |format| format.html # show.html.haml diff --git a/app/views/posts/show.html.haml b/app/views/posts/show.html.haml index c54fd1e1a..532b93b15 100644 --- a/app/views/posts/show.html.haml +++ b/app/views/posts/show.html.haml @@ -12,10 +12,10 @@ %a{:name => "comments"} -- if @post.comments.length > 0 +- if @comments %h2 - =pluralize(@post.comments.length, "comment") - - @post.comments.each do |c| + =pluralize(@comments.length, "comment") + - @comments.each do |c| = render :partial => "comments/single", :locals => { :comment => c } - else diff --git a/spec/views/posts/show.html.haml_spec.rb b/spec/views/posts/show.html.haml_spec.rb index ee983c702..34c21f9e0 100644 --- a/spec/views/posts/show.html.haml_spec.rb +++ b/spec/views/posts/show.html.haml_spec.rb @@ -46,6 +46,7 @@ describe "posts/show" do @post = assign(:post, FactoryGirl.create(:html_post, :author => @author)) @comment = FactoryGirl.create(:comment, :post => @post) + @comments = @post.comments render end @@ -73,6 +74,7 @@ describe "posts/show" do @comment4 = FactoryGirl.create(:comment, :post => @post, :body => "F0urth!!!") @comment2 = FactoryGirl.create(:comment, :post => @post, :body => "S3c0nd!!1!", :created_at => Date.new(2011, 5, 17)) + @comments = @post.comments render end From da4a86bd2cf10b18aecdc2c88217d4aefa196dbe Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 29 Mar 2013 10:25:28 +1100 Subject: [PATCH 06/23] Hotfix: don't escape HTML for analytics script --- app/views/layouts/application.html.haml | 2 +- spec/views/layouts/application_spec.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index d8a8088e4..cc67cedee 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -25,4 +25,4 @@ / Placed at the end of the document so the pages load faster = javascript_include_tag "application" - = Growstuff::Application.config.analytics_code + != Growstuff::Application.config.analytics_code diff --git a/spec/views/layouts/application_spec.rb b/spec/views/layouts/application_spec.rb index 51db99ef3..48cd360ce 100644 --- a/spec/views/layouts/application_spec.rb +++ b/spec/views/layouts/application_spec.rb @@ -58,9 +58,10 @@ describe 'layouts/application.html.haml', :type => "view" do end it 'includes the analytics code' do - Growstuff::Application.config.analytics_code = 'ANALYTICS' + Growstuff::Application.config.analytics_code = '' render - rendered.should contain 'ANALYTICS' + assert_select "script", :text => 'alert("foo!")' + rendered.should_not contain 'script' end end From 0e9b4caf5893676651f4d2bbaddf62f76db2b04d Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 29 Mar 2013 16:33:37 +1100 Subject: [PATCH 07/23] added sunniness field to plantings --- app/models/planting.rb | 3 +++ .../20130329045744_add_sunniness_to_planting.rb | 5 +++++ db/schema.rb | 3 ++- spec/factories/planting.rb | 1 + spec/models/planting_spec.rb | 17 +++++++++++++++++ 5 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20130329045744_add_sunniness_to_planting.rb diff --git a/app/models/planting.rb b/app/models/planting.rb index 311d711cf..df83ab2b8 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -13,6 +13,9 @@ class Planting < ActiveRecord::Base :to => :crop, :prefix => true + validates :sunniness, :inclusion => { :in => %w(sun semi-shade shade), + :message => "%{value} is not a valid sunniness value" } + def planting_slug "#{owner.login_name}-#{garden}-#{crop}".downcase.gsub(' ', '-') end diff --git a/db/migrate/20130329045744_add_sunniness_to_planting.rb b/db/migrate/20130329045744_add_sunniness_to_planting.rb new file mode 100644 index 000000000..8fae8d1d0 --- /dev/null +++ b/db/migrate/20130329045744_add_sunniness_to_planting.rb @@ -0,0 +1,5 @@ +class AddSunninessToPlanting < ActiveRecord::Migration + def change + add_column :plantings, :sunniness, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 4b23284c5..c652f0a99 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 => 20130327120024) do +ActiveRecord::Schema.define(:version => 20130329045744) do create_table "comments", :force => true do |t| t.integer "post_id", :null => false @@ -130,6 +130,7 @@ ActiveRecord::Schema.define(:version => 20130327120024) do t.datetime "created_at", :null => false t.datetime "updated_at", :null => false t.string "slug" + t.string "sunniness" end add_index "plantings", ["slug"], :name => "index_plantings_on_slug", :unique => true diff --git a/spec/factories/planting.rb b/spec/factories/planting.rb index 6cda6ab21..531247609 100644 --- a/spec/factories/planting.rb +++ b/spec/factories/planting.rb @@ -5,5 +5,6 @@ FactoryGirl.define do planted_at Time.now quantity 33 description "This is a *really* good plant." + sunniness 'sun' end end diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 53cc1b7fb..5ed234894 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -38,4 +38,21 @@ describe Planting do @planting.planted_at_string.should == "2013-03-01" end + it 'should have a sunniness value' do + @planting.sunniness.should eq 'sun' + end + + it 'all three valid sunniness values should work' do + ['sun', 'shade', 'semi-shade'].each do |s| + @planting = FactoryGirl.build(:planting, :sunniness => s) + @planting.should be_valid + end + end + + it 'should refuse invalid sunniness values' do + @planting = FactoryGirl.build(:planting, :sunniness => 'not valid') + @planting.should_not be_valid + @planting.errors[:sunniness].should include("not valid is not a valid sunniness value") + end + end From 2bfe033a0af0c4298c62d4ef616e8c31e2c19f64 Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 29 Mar 2013 16:58:16 +1100 Subject: [PATCH 08/23] generalised sunniness into a variable for later reuse --- app/models/planting.rb | 10 ++++++---- spec/models/planting_spec.rb | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index df83ab2b8..0c800cba5 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -2,8 +2,8 @@ class Planting < ActiveRecord::Base extend FriendlyId friendly_id :planting_slug, use: :slugged - attr_accessible :crop_id, :description, :garden_id, :planted_at, :quantity, - :planted_at_string + attr_accessible :crop_id, :description, :garden_id, :planted_at, + :quantity, :sunniness, :planted_at_string belongs_to :garden belongs_to :crop @@ -13,8 +13,10 @@ class Planting < ActiveRecord::Base :to => :crop, :prefix => true - validates :sunniness, :inclusion => { :in => %w(sun semi-shade shade), - :message => "%{value} is not a valid sunniness value" } + SUNNINESS_VALUES = %w(sun semi-shade shade) + validates :sunniness, :inclusion => { :in => SUNNINESS_VALUES, + :message => "%{value} is not a valid sunniness value" }, + :allow_nil => true def planting_slug "#{owner.login_name}-#{garden}-#{crop}".downcase.gsub(' ', '-') diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 5ed234894..ee2f26346 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -43,7 +43,7 @@ describe Planting do end it 'all three valid sunniness values should work' do - ['sun', 'shade', 'semi-shade'].each do |s| + ['sun', 'shade', 'semi-shade', nil].each do |s| @planting = FactoryGirl.build(:planting, :sunniness => s) @planting.should be_valid end From 2770b90e3add4651aa83175fb67f2d2c59c24349 Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 29 Mar 2013 17:01:14 +1100 Subject: [PATCH 09/23] allow blank sunniness value as well as nil --- app/models/planting.rb | 3 ++- spec/models/planting_spec.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index 0c800cba5..5fd3e86a2 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -16,7 +16,8 @@ class Planting < ActiveRecord::Base SUNNINESS_VALUES = %w(sun semi-shade shade) validates :sunniness, :inclusion => { :in => SUNNINESS_VALUES, :message => "%{value} is not a valid sunniness value" }, - :allow_nil => true + :allow_nil => true, + :allow_blank => true def planting_slug "#{owner.login_name}-#{garden}-#{crop}".downcase.gsub(' ', '-') diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index ee2f26346..591932385 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -43,7 +43,7 @@ describe Planting do end it 'all three valid sunniness values should work' do - ['sun', 'shade', 'semi-shade', nil].each do |s| + ['sun', 'shade', 'semi-shade', nil, ''].each do |s| @planting = FactoryGirl.build(:planting, :sunniness => s) @planting.should be_valid end From 9b179553cea486fd319f64d7720957bdfb54319e Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 29 Mar 2013 17:02:01 +1100 Subject: [PATCH 10/23] added sunniness to the planting form --- app/views/plantings/_form.html.haml | 4 ++++ spec/views/plantings/new.html.haml_spec.rb | 1 + 2 files changed, 5 insertions(+) diff --git a/app/views/plantings/_form.html.haml b/app/views/plantings/_form.html.haml index 092ca9de0..2a8906add 100644 --- a/app/views/plantings/_form.html.haml +++ b/app/views/plantings/_form.html.haml @@ -20,6 +20,10 @@ = f.label 'How many?', :class => 'control-label' .controls = f.number_field :quantity, :class => 'input-small' + .control-group + = f.label 'Sun or shade?', :class => 'control-label' + .controls + = f.select(:sunniness, Planting::SUNNINESS_VALUES, {:include_blank => true}) .control-group = f.label 'Tell us more about it', :class => 'control-label' .controls= f.text_area :description, :rows => 6 diff --git a/spec/views/plantings/new.html.haml_spec.rb b/spec/views/plantings/new.html.haml_spec.rb index d57208dd8..a07273297 100644 --- a/spec/views/plantings/new.html.haml_spec.rb +++ b/spec/views/plantings/new.html.haml_spec.rb @@ -33,6 +33,7 @@ describe "plantings/new" do assert_select "select#planting_crop_id", :name => "planting[crop_id]" assert_select "input#planting_quantity", :name => "planting[quantity]" assert_select "textarea#planting_description", :name => "planting[description]" + assert_select "select#planting_sunniness", :name => "planting[sunniness]" end end From 8140671525a9115d4d71dea1632581ad992ab5d8 Mon Sep 17 00:00:00 2001 From: Skud Date: Fri, 29 Mar 2013 17:13:28 +1100 Subject: [PATCH 11/23] display sunniness on planting page --- app/views/plantings/show.html.haml | 6 ++++++ spec/views/plantings/show.html.haml_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/app/views/plantings/show.html.haml b/app/views/plantings/show.html.haml index 77b59f59f..9907aedd7 100644 --- a/app/views/plantings/show.html.haml +++ b/app/views/plantings/show.html.haml @@ -15,6 +15,12 @@ %b Quantity: = @planting.quantity != 0 ? @planting.quantity : "not specified" + - if ! @planting.sunniness.blank? + %p + %b Sun or shade? + = @planting.sunniness + + - if can? :edit, @planting or can? :destroy, @planting %p - if can? :edit, @planting diff --git a/spec/views/plantings/show.html.haml_spec.rb b/spec/views/plantings/show.html.haml_spec.rb index 7afb70cda..8eabcd535 100644 --- a/spec/views/plantings/show.html.haml_spec.rb +++ b/spec/views/plantings/show.html.haml_spec.rb @@ -9,6 +9,26 @@ describe "plantings/show" do ) end + it "shows the sunniness" do + controller.stub(:current_user) { nil } + @member = FactoryGirl.create(:member) + create_planting_for(@member) + render + rendered.should contain 'Sun or shade?' + rendered.should contain 'sun' + end + + it "doesn't show sunniness if blank" do + controller.stub(:current_user) { nil } + @member = FactoryGirl.create(:member) + @p = create_planting_for(@member) + @p.sunniness = '' + @p.save + render + rendered.should_not contain 'Sun or shade?' + rendered.should_not contain 'sun' + end + context "no location set" do before(:each) do controller.stub(:current_user) { nil } From 05d1a8807a20bbf60ff48b4ff9d8988bcead102b Mon Sep 17 00:00:00 2001 From: Skud Date: Sun, 31 Mar 2013 20:28:29 +1100 Subject: [PATCH 12/23] moved Member.confirmed to a scope --- app/controllers/members_controller.rb | 2 +- app/models/member.rb | 10 ++-------- spec/models/member_spec.rb | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 24225d3f3..e1e63f8bc 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -9,7 +9,7 @@ class MembersController < ApplicationController end def show - @member = Member.find_confirmed(params[:id]) + @member = Member.confirmed.find(params[:id]) @posts = @member.posts # The garden form partial is called from the "New Garden" tab; # it requires a garden to be passed in @garden. diff --git a/app/models/member.rb b/app/models/member.rb index 445533947..9877eaf06 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -10,7 +10,9 @@ class Member < ActiveRecord::Base has_and_belongs_to_many :roles has_many :notifications, :foreign_key => 'recipient_id' has_many :sent_notifications, :foreign_key => 'sender_id' + default_scope order("lower(login_name) asc") + scope :confirmed, where('confirmed_at IS NOT NULL') # Include default devise modules. Others available are: # :token_authenticatable, :confirmable, @@ -68,14 +70,6 @@ class Member < ActiveRecord::Base end end - def self.find_confirmed(params) - find(params, :conditions => 'confirmed_at IS NOT NULL') - end - - def self.confirmed - where('confirmed_at IS NOT NULL') - end - def to_s return login_name end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index e8ef4baad..e08052a3b 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -199,4 +199,22 @@ describe 'member' do end end + context 'confirmed scope' do + before(:each) do + @member1 = FactoryGirl.create(:member) + @member2 = FactoryGirl.create(:member) + end + + it 'sees confirmed members' do + Member.confirmed.count.should == 2 + end + + it 'ignores unconfirmed members' do + @member3 = FactoryGirl.create(:unconfirmed_member) + Member.confirmed.count.should == 2 + end + + + end + end From a5f6931e1214ded60f6b7d1f877bfb9b3033b52a Mon Sep 17 00:00:00 2001 From: Skud Date: Sun, 31 Mar 2013 21:00:57 +1100 Subject: [PATCH 13/23] rewrote blurb --- app/controllers/home_controller.rb | 10 ++++++++ app/views/home/_blurb.html.haml | 17 ++++++++++--- app/views/home/index.html.haml | 39 ------------------------------ spec/views/home/index_spec.rb | 1 - 4 files changed, 23 insertions(+), 44 deletions(-) diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 26348c670..a0aab0f29 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -1,5 +1,15 @@ class HomeController < ApplicationController skip_authorize_resource + def index + @member_count = Member.confirmed.count + @crop_count = Crop.count + @planting_count = Planting.count + @garden_count = Garden.count + + respond_to do |format| + format.html # index.html.haml + end end + end diff --git a/app/views/home/_blurb.html.haml b/app/views/home/_blurb.html.haml index 9a07fe558..f5f4b4676 100644 --- a/app/views/home/_blurb.html.haml +++ b/app/views/home/_blurb.html.haml @@ -1,6 +1,15 @@ -%h1 - = Growstuff::Application.config.site_name +%h1= Growstuff::Application.config.site_name -%p #{Growstuff::Application.config.site_name} is a community of food gardeners working together to build an open source platform to track, share, and discuss edible gardens and sustainable lifestyles. You can join us right now and be part of growing our website, from seed to harvest. We welcome you regardless of your experience, and invite you to be part of our development process. +%p + #{Growstuff::Application.config.site_name} is a community of food gardeners. We're building an open source platform to track, share, and discuss edible gardens and sustainable lifestyles. We offer growing information tailored to your location, and help you connect with your local food-growing community. -%p= link_to 'Learn more', 'http://wiki.growstuff.org/', :class => 'btn btn-primary btn-large' + %strong + So far, + = link_to "#{@member_count} members", members_path + have planted + = link_to "#{@crop_count} crops", crops_path + #{@planting_count} times in #{@garden_count} gardens. + + You could be one of them. Sign up for a free account and start tracking your food garden today. +%p + = link_to 'Join now', new_member_registration_path, :class => 'btn btn-primary btn-large' diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml index 8b2e9f96c..7596b8b21 100644 --- a/app/views/home/index.html.haml +++ b/app/views/home/index.html.haml @@ -73,48 +73,9 @@ - if can? :create, Planting %p= link_to "Post something", new_post_path, :class => 'btn btn-primary' - .span6 - - - else .visible-desktop.visible-tablet .hero-unit = render :partial => 'blurb' - .visible-phone = render :partial => 'blurb' - - .row - .span3 - %h2 Track - %p - Track your garden, your - = link_to 'crops', crops_path - , and everything to do with them. | - Remember what you planted or harvested, and plan for what you want to do next. | - Import and export your data to a spreadsheet or other format. - - .span3 - %h2 Connect - %p - Connect with friends, people who live in your area, or others | - who share your gardening interests. Show off your crops and | - harvests, share updates and tips, and - = link_to 'see', posts_path - what everyone else is doing, too. - .span3 - %h2 Learn - %p - Browse our - = link_to 'crops', crops_path - database and learn about food plants for your - climate and garden type. Read other - = link_to "members' posts", posts_path - and tips, or post questions and discuss food gardening issues in our forums. - .span3 - %h2 Trade - %p - Use #{ Growstuff::Application.config.site_name } as a marketplace - for everything related to food gardens. Buy and sell, trade, - or give away anything from seeds to garden supplies, or find - events and businesses in your area. diff --git a/spec/views/home/index_spec.rb b/spec/views/home/index_spec.rb index 38b4a4fb8..f70cdcb18 100644 --- a/spec/views/home/index_spec.rb +++ b/spec/views/home/index_spec.rb @@ -8,7 +8,6 @@ describe 'home/index.html.haml', :type => "view" do it 'should have description' do rendered.should contain 'is a community of food gardeners' - rendered.should contain 'We welcome you regardless of your experience, and invite you to be part of our development process.' end end From e64776afc29ca299b51810853ea0512ef7775d21 Mon Sep 17 00:00:00 2001 From: Skud Date: Sun, 31 Mar 2013 21:19:30 +1100 Subject: [PATCH 14/23] sort plantings by reverse creation date --- app/models/planting.rb | 2 ++ spec/models/planting_spec.rb | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/app/models/planting.rb b/app/models/planting.rb index 311d711cf..d2554ac44 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -8,6 +8,8 @@ class Planting < ActiveRecord::Base belongs_to :garden belongs_to :crop + default_scope order("created_at desc") + delegate :default_scientific_name, :plantings_count, :to => :crop, diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 53cc1b7fb..cce9cf65f 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -38,4 +38,9 @@ describe Planting do @planting.planted_at_string.should == "2013-03-01" end + it 'should sort in reverse creation order' do + @planting2 = FactoryGirl.create(:planting) + Planting.first.should eq @planting2 + end + end From fe895562f5dc4d9eea6da16f2058406292691883 Mon Sep 17 00:00:00 2001 From: gnattery Date: Mon, 1 Apr 2013 11:34:05 +1100 Subject: [PATCH 15/23] a few small tweaks to the views --- app/views/notifications/_form.html.haml | 6 +++--- app/views/notifications/new.html.haml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/notifications/_form.html.haml b/app/views/notifications/_form.html.haml index d00d6848b..6eccf0faa 100644 --- a/app/views/notifications/_form.html.haml +++ b/app/views/notifications/_form.html.haml @@ -11,11 +11,11 @@ = f.hidden_field :sender_id, :value => @sender.id %p - = label_tag "You are sending a private message to: " + To: = link_to @recipient, @recipient - = label_tag :notification, "Subject" + = label_tag :notification, "Subject:" = f.text_field :subject, :class => 'input-block-level' - = label_tag :body, "Type your message here" + = label_tag :body, "Type your message here:" = f.text_area :body, :rows => 12, :class => 'input-block-level' %span.help-block = render :partial => "shared/markdown_help" diff --git a/app/views/notifications/new.html.haml b/app/views/notifications/new.html.haml index 87f89de3c..f34343133 100644 --- a/app/views/notifications/new.html.haml +++ b/app/views/notifications/new.html.haml @@ -1,4 +1,4 @@ -= content_for :title, "Send a message" += content_for :title, "Send a message to #{@recipient}" =render 'form' From ceef39e846ace4ba5e7e3232e98e5ac31191eb2f Mon Sep 17 00:00:00 2001 From: gnattery Date: Mon, 1 Apr 2013 11:44:14 +1100 Subject: [PATCH 16/23] Cleaned up controller for notifications. Removed json from notification, since it's a bit risky and we don't really need it. --- app/controllers/notifications_controller.rb | 15 ++------------- app/views/notifications/_form.html.haml | 1 - spec/controllers/notifications_controller_spec.rb | 11 +---------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 6cc9c13df..7b49c326f 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,18 +1,15 @@ class NotificationsController < ApplicationController load_and_authorize_resource # GET /notifications - # GET /notifications.json def index @notifications = Notification.find_all_by_recipient_id(current_member) respond_to do |format| format.html # index.html.erb - format.json { render json: @notifications } end end # GET /notifications/1 - # GET /notifications/1.json def show @notification = Notification.find(params[:id]) @notification.read = true @@ -20,49 +17,41 @@ class NotificationsController < ApplicationController respond_to do |format| format.html # show.html.erb - format.json { render json: @notification } end end # GET /notifications/new - # GET /notifications/new.json def new @notification = Notification.new @recipient = Member.find_by_id(params[:recipient_id]) - @sender = Member.find_by_id(params[:sender_id]) respond_to do |format| format.html # new.html.erb - format.json { render json: @notification } end end # DELETE /notifications/1 - # DELETE /notifications/1.json def destroy @notification = Notification.find(params[:id]) @notification.destroy respond_to do |format| format.html { redirect_to notifications_url } - format.json { head :no_content } end end # POST /notifications - # POST /notifications.json def create + params[:notification][:sender_id] = current_member.id @notification = Notification.new(params[:notification]) @recipient = Member.find_by_id(params[:notification][:recipient_id]) - + respond_to do |format| if @notification.save format.html { redirect_to @recipient, notice: 'Message was successfully sent.' } - format.json { render json: @notification, status: :created, location: @notification } else format.html { render action: "new" } - format.json { render json: @notification.errors, status: :unprocessable_entity } end end end diff --git a/app/views/notifications/_form.html.haml b/app/views/notifications/_form.html.haml index 6eccf0faa..a0b9f6d55 100644 --- a/app/views/notifications/_form.html.haml +++ b/app/views/notifications/_form.html.haml @@ -8,7 +8,6 @@ .field = f.hidden_field :recipient_id, :value => @recipient.id - = f.hidden_field :sender_id, :value => @sender.id %p To: diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index a1642b36f..e015c531a 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -47,13 +47,6 @@ describe NotificationsController do get :new, {:recipient_id => @recipient.id } assigns(:recipient).should be_an_instance_of(Member) end - - it "assigns a sender" do - @sender = FactoryGirl.create(:member) - @notification = FactoryGirl.create(:notification, :sender => @sender) - get :new, {:sender_id => @sender.id } - assigns(:sender).should be_an_instance_of(Member) - end end describe "DELETE destroy" do @@ -86,10 +79,8 @@ describe NotificationsController do end it "redirects to the recipient's profile" do - @sender = FactoryGirl.create(:member) @recipient = FactoryGirl.create(:member) - post :create, { :notification => { :recipient_id => @recipient.id, - :sender_id => @sender.id}} + post :create, { :notification => { :recipient_id => @recipient.id}} response.should redirect_to(@recipient) end end From b2198101fa8954121f978263d9a5885c1a061a3d Mon Sep 17 00:00:00 2001 From: gnattery Date: Mon, 1 Apr 2013 12:26:22 +1100 Subject: [PATCH 17/23] using cancan more correctly, fixed broken tests --- app/models/ability.rb | 8 +++-- app/views/members/show.html.haml | 3 +- .../notifications_controller_spec.rb | 21 +++++++++-- spec/models/ability_spec.rb | 36 ++++++++++++++----- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index b508f4f8d..c5e8c4905 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -29,8 +29,12 @@ class Ability # can read/delete notifications that were sent to them can :read, Notification, :recipient_id => member.id can :destroy, Notification, :recipient_id => member.id - # can create notifications that were sent by them - can :create, Notification, :sender_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 # only crop wranglers can create/edit/destroy crops diff --git a/app/views/members/show.html.haml b/app/views/members/show.html.haml index 6e7e65e11..fb824824f 100644 --- a/app/views/members/show.html.haml +++ b/app/views/members/show.html.haml @@ -6,8 +6,9 @@ .span3 = render :partial => "members/avatar", :locals => { :member => @member } - -if current_member && current_member != @member + -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' %p diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index e015c531a..e30b247bf 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -5,7 +5,22 @@ describe NotificationsController do login_member def valid_attributes - { "recipient_id" => subject.current_member.id } + { + "recipient_id" => subject.current_member.id, + "sender_id" => FactoryGirl.create(:member).id + } + end + + # this gets a bit confused because for most of the notification tests + # (reading, etc) the logged in member needs to be the recipient. + # However, for sending private messages (create, etc) the logged in + # member needs to be the sender. Hence this separate set of + # attributes. + def valid_attributes_for_sender + { + "sender_id" => subject.current_member.id, + "recipient_id" => FactoryGirl.create(:member).id + } end def valid_session @@ -68,12 +83,12 @@ describe NotificationsController do describe "with valid params" do it "creates a new notification" do expect { - post :create, {:notification => valid_attributes} + post :create, {:notification => valid_attributes_for_sender} }.to change(Notification, :count).by(1) end it "assigns a newly created notification as @notification" do - post :create, {:notification => valid_attributes} + post :create, {:notification => valid_attributes_for_sender} assigns(:notification).should be_a(Notification) assigns(:notification).should be_persisted end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index f89574629..04d710ef5 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -7,16 +7,34 @@ describe Ability do @ability = Ability.new(@member) end - it 'member can view their own notifications' do - @notification = FactoryGirl.create(:notification, :recipient => @member) - @ability.should be_able_to(:read, @notification) - end + context "notifications" do + it 'member can view their own notifications' do + @notification = FactoryGirl.create(:notification, :recipient => @member) + @ability.should be_able_to(:read, @notification) + end - it "member can't view someone else's notifications" do - @notification = FactoryGirl.create(:notification, - :recipient => FactoryGirl.create(:member) - ) - @ability.should_not be_able_to(:read, @notification) + it "member can't view someone else's notifications" do + @notification = FactoryGirl.create(:notification, + :recipient => FactoryGirl.create(:member) + ) + @ability.should_not be_able_to(:read, @notification) + end + it "member can't send messages to themself" do + @ability.should_not be_able_to(:create, + FactoryGirl.create(:notification, + :recipient => @member, + :sender => @member + ) + ) + end + it "member can send messages to someone else" do + @ability.should be_able_to(:create, + FactoryGirl.create(:notification, + :recipient => FactoryGirl.create(:member), + :sender => @member + ) + ) + end end context "crop wrangling" do From 759903761dfea28093959d5964baba8bc87cd493 Mon Sep 17 00:00:00 2001 From: gnattery Date: Mon, 1 Apr 2013 15:53:41 +1100 Subject: [PATCH 18/23] sort plantings --- app/models/planting.rb | 2 ++ db/schema.rb | 9 --------- spec/models/planting_spec.rb | 6 ++++++ 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index 5fd3e86a2..ab312f122 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -13,6 +13,8 @@ class Planting < ActiveRecord::Base :to => :crop, :prefix => true + default_scope order("created_at desc") + SUNNINESS_VALUES = %w(sun semi-shade shade) validates :sunniness, :inclusion => { :in => SUNNINESS_VALUES, :message => "%{value} is not a valid sunniness value" }, diff --git a/db/schema.rb b/db/schema.rb index c652f0a99..b354a3da0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -112,15 +112,6 @@ ActiveRecord::Schema.define(:version => 20130329045744) do t.datetime "updated_at", :null => false end - create_table "payments", :force => true do |t| - t.integer "payer_id" - t.decimal "amount" - t.date "paid_period_begins" - t.date "paid_period_ends" - 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/planting_spec.rb b/spec/models/planting_spec.rb index 591932385..8dcffbdbd 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -19,6 +19,12 @@ describe Planting do @planting.location.should match /^member\d+'s Springfield Community Garden$/ end + it "sorts plantings in descending order of creation" do + @planting1 = FactoryGirl.create(:planting) + @planting2 = FactoryGirl.create(:planting) + Planting.first.should eq @planting2 + end + it "should have a slug" do @planting.slug.should match /^member\d+-springfield-community-garden-tomato$/ end From 2373bbf5c929ab7196b1741601c35bf0c858cb7a Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 2 Apr 2013 12:15:08 +1100 Subject: [PATCH 19/23] Broke recent plantings/posts out into partial ... which is now displayed on both the signed in and signed out homepages. --- app/controllers/home_controller.rb | 10 ++++++ app/views/home/index.html.haml | 36 ++++++-------------- app/views/shared/_recent_plantings.html.haml | 12 +++++++ app/views/shared/_recent_posts.html.haml | 17 +++++++++ spec/controllers/home_controller_spec.rb | 33 ++++++++++++++++++ spec/views/home/index_spec.rb | 4 +++ 6 files changed, 86 insertions(+), 26 deletions(-) create mode 100644 app/views/shared/_recent_plantings.html.haml create mode 100644 app/views/shared/_recent_posts.html.haml create mode 100644 spec/controllers/home_controller_spec.rb diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index a0aab0f29..f6eb11b30 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -7,6 +7,16 @@ class HomeController < ApplicationController @planting_count = Planting.count @garden_count = Garden.count + # customise what we show on the homepage based on whether you're + # logged in or not. + @member = current_member + @plantings = current_member ? + current_member.plantings.limit(10) : + Planting.limit(10) + @posts = current_member ? + current_member.posts.limit(10) : + Post.limit(10) + respond_to do |format| format.html # index.html.haml end diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml index 7596b8b21..27ae6a4e3 100644 --- a/app/views/home/index.html.haml +++ b/app/views/home/index.html.haml @@ -43,35 +43,11 @@ .row .span6 %h2 Your recent plantings - - if current_member.plantings.count > 0 - %ul - - current_member.plantings.limit(10).each do |p| - %li - = link_to "#{p.crop.system_name} in #{p.location}", p - - if p.planted_at - on - = p.planted_at.to_s(:date) - - else - %p None yet. - - if can? :create, Planting - %p= link_to "Plant something", new_planting_path, :class => 'btn btn-primary' + = render :partial => 'shared/recent_plantings' .span6 %h2 Your recent posts - - if current_member.posts.count > 0 - %ul - - current_member.posts.limit(10).each do |p| - %li - = link_to p.subject, p - - if p.forum - in - = link_to p.forum.name, p.forum - on - = p.created_at.to_s(:date) - - else - %p None yet. - - if can? :create, Planting - %p= link_to "Post something", new_post_path, :class => 'btn btn-primary' + = render :partial => 'shared/recent_posts' - else .visible-desktop.visible-tablet @@ -79,3 +55,11 @@ = render :partial => 'blurb' .visible-phone = render :partial => 'blurb' + + .row + .span6 + %h2 Recent plantings + = render :partial => 'shared/recent_plantings' + .span6 + %h2 Recent posts + = render :partial => 'shared/recent_posts' diff --git a/app/views/shared/_recent_plantings.html.haml b/app/views/shared/_recent_plantings.html.haml new file mode 100644 index 000000000..3d60a12e8 --- /dev/null +++ b/app/views/shared/_recent_plantings.html.haml @@ -0,0 +1,12 @@ +- if @plantings + %ul + - @plantings.each do |p| + %li + = link_to "#{p.crop.system_name} in #{p.location}", p + - if p.planted_at + on + = p.planted_at.to_s(:date) +- else + %p None yet. +- if can? :create, Planting + %p= link_to "Plant something", new_planting_path, :class => 'btn btn-primary' diff --git a/app/views/shared/_recent_posts.html.haml b/app/views/shared/_recent_posts.html.haml new file mode 100644 index 000000000..82d06a2d7 --- /dev/null +++ b/app/views/shared/_recent_posts.html.haml @@ -0,0 +1,17 @@ +- if @posts + %ul + - @posts.each do |p| + %li + = link_to p.subject, p + - unless @member + posted by + = link_to p.author, p.author + - if p.forum + in + = link_to p.forum.name, p.forum + on + = p.created_at.to_s(:date) +- else + %p None yet. +- if can? :create, Post + %p= link_to "Post something", new_post_path, :class => 'btn btn-primary' diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb new file mode 100644 index 000000000..2f5668a8d --- /dev/null +++ b/spec/controllers/home_controller_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe HomeController do + + describe "GET index" do + it "assigns counts" do + @planting = FactoryGirl.create(:planting) + get :index, {} + assigns(:garden_count).should == 2 # auto-created for member and planting + assigns(:planting_count).should == 1 + assigns(:crop_count).should == 1 + assigns(:member_count).should == 1 + end + + it "assigns posts and plantings" do + @post = FactoryGirl.create(:post) + @planting = FactoryGirl.create(:planting) + get :index, {} + assigns(:posts).should eq [@post] + assigns(:plantings).should eq [@planting] + end + + context 'logged in' do + + login_member + + it 'assigns member' do + get :index, {} + assigns(:member).should be_an_instance_of Member + end + end + end +end diff --git a/spec/views/home/index_spec.rb b/spec/views/home/index_spec.rb index f70cdcb18..2fff1bb20 100644 --- a/spec/views/home/index_spec.rb +++ b/spec/views/home/index_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe 'home/index.html.haml', :type => "view" do context 'logged out' do before(:each) do + controller.stub(:current_user) { nil } render end @@ -16,11 +17,14 @@ describe 'home/index.html.haml', :type => "view" do @member = FactoryGirl.create(:geolocated_member) controller.stub(:current_user) { @member } sign_in @member + assign(:member, @member) @planting = FactoryGirl.create(:planting, :garden => @member.gardens.first ) + assign(:plantings, [@planting]) @forum = FactoryGirl.create(:forum, :owner => @member) @post = FactoryGirl.create(:post, :author => @member) + assign(:posts, [@post]) @role = FactoryGirl.create(:admin) @member.roles << @role render From 0ec01ab259be5a3a345ba724fd36048404e38512 Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 2 Apr 2013 12:30:05 +1100 Subject: [PATCH 20/23] improved date/time formatting we set defaults for date and time formatting (and override them in a few places) --- app/views/notifications/index.html.haml | 4 ++-- app/views/notifications/show.html.haml | 2 +- app/views/notifier/notify.html.haml | 2 +- app/views/plantings/_thumbnail.html.haml | 2 +- app/views/plantings/show.html.haml | 2 +- app/views/shared/_recent_plantings.html.haml | 2 +- config/initializers/time_formats.rb | 6 ++++++ spec/factories/planting.rb | 2 +- spec/views/plantings/_thumbnail.html.haml_spec.rb | 2 +- 9 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 3352120cd..44b408ce0 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -23,9 +23,9 @@ %strong= link_to n.subject, notification_path(n) %td - if n.read - = n.created_at.to_s(:date) + = n.created_at - else - %strong= n.created_at.to_s(:date) + %strong= n.created_at %td = link_to 'Delete', n, method: :delete, data: { confirm: 'Are you sure?' }, :class => 'btn btn-mini' - else diff --git a/app/views/notifications/show.html.haml b/app/views/notifications/show.html.haml index f9a1870ed..cbd66aeaf 100644 --- a/app/views/notifications/show.html.haml +++ b/app/views/notifications/show.html.haml @@ -4,7 +4,7 @@ From = link_to @notification.sender, @notification.sender on - = @notification.created_at.to_s(:date) + = @notification.created_at - if @notification.post_id in response to: diff --git a/app/views/notifier/notify.html.haml b/app/views/notifier/notify.html.haml index 80ad294cf..cbbb82bcd 100644 --- a/app/views/notifier/notify.html.haml +++ b/app/views/notifier/notify.html.haml @@ -4,7 +4,7 @@ %p You have received a message from = link_to @notification.sender.login_name, member_url(@notification.sender) - on #{site_name} at #{@notification.created_at.to_s(:date)} + on #{site_name} at #{@notification.created_at} - if @notification.post in response to = link_to @notification.post.subject, post_url(@notification.post) diff --git a/app/views/plantings/_thumbnail.html.haml b/app/views/plantings/_thumbnail.html.haml index e5338ccd8..2e6147f02 100644 --- a/app/views/plantings/_thumbnail.html.haml +++ b/app/views/plantings/_thumbnail.html.haml @@ -8,7 +8,7 @@ %p Planted - if planting.planted_at - = planting.planted_at.to_s(:date) + = planting.planted_at in = link_to planting.location, planting.garden diff --git a/app/views/plantings/show.html.haml b/app/views/plantings/show.html.haml index 77b59f59f..e5293cb67 100644 --- a/app/views/plantings/show.html.haml +++ b/app/views/plantings/show.html.haml @@ -4,7 +4,7 @@ .span6 %p %b Planted: - = @planting.planted_at ? @planting.planted_at.to_s(:date) : "not specified" + = @planting.planted_at ? @planting.planted_at : "not specified" %p %b Where: =link_to "#{@planting.owner}'s", @planting.owner diff --git a/app/views/shared/_recent_plantings.html.haml b/app/views/shared/_recent_plantings.html.haml index 3d60a12e8..6dc12570b 100644 --- a/app/views/shared/_recent_plantings.html.haml +++ b/app/views/shared/_recent_plantings.html.haml @@ -5,7 +5,7 @@ = link_to "#{p.crop.system_name} in #{p.location}", p - if p.planted_at on - = p.planted_at.to_s(:date) + = p.planted_at - else %p None yet. - if can? :create, Planting diff --git a/config/initializers/time_formats.rb b/config/initializers/time_formats.rb index 63c12499e..0b26541d9 100644 --- a/config/initializers/time_formats.rb +++ b/config/initializers/time_formats.rb @@ -1 +1,7 @@ +Time::DATE_FORMATS[:default] = '%B %d, %Y at %H:%M' +Date::DATE_FORMATS[:default] = "%B %d, %Y" + Time::DATE_FORMATS[:date] = "%B %d, %Y" +Date::DATE_FORMATS[:date] = "%B %d, %Y" + +Time::DATE_FORMATS[:datetime] = '%B %d, %Y at %H:%M' diff --git a/spec/factories/planting.rb b/spec/factories/planting.rb index 6cda6ab21..7185edacb 100644 --- a/spec/factories/planting.rb +++ b/spec/factories/planting.rb @@ -2,7 +2,7 @@ FactoryGirl.define do factory :planting do garden crop - planted_at Time.now + planted_at Date.today quantity 33 description "This is a *really* good plant." end diff --git a/spec/views/plantings/_thumbnail.html.haml_spec.rb b/spec/views/plantings/_thumbnail.html.haml_spec.rb index 3e55d08ea..5abebe8c7 100644 --- a/spec/views/plantings/_thumbnail.html.haml_spec.rb +++ b/spec/views/plantings/_thumbnail.html.haml_spec.rb @@ -25,7 +25,7 @@ describe "plantings/_thumbnail" do end it "renders the date planted" do - rendered.should contain @planting.planted_at.to_s(:date) + rendered.should contain @planting.planted_at.to_s(:default) end it "shows the name of the crop" do From 93b173a2728074a7cbf35e6b75373a76b435d15a Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 2 Apr 2013 12:39:07 +1100 Subject: [PATCH 21/23] delegate crop system name and url --- app/models/planting.rb | 4 +++- app/views/shared/_recent_plantings.html.haml | 2 +- spec/models/planting_spec.rb | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index d2554ac44..c55c4ac13 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -10,7 +10,9 @@ class Planting < ActiveRecord::Base default_scope order("created_at desc") - delegate :default_scientific_name, + delegate :system_name, + :en_wikipedia_url, + :default_scientific_name, :plantings_count, :to => :crop, :prefix => true diff --git a/app/views/shared/_recent_plantings.html.haml b/app/views/shared/_recent_plantings.html.haml index 6dc12570b..4fda76254 100644 --- a/app/views/shared/_recent_plantings.html.haml +++ b/app/views/shared/_recent_plantings.html.haml @@ -2,7 +2,7 @@ %ul - @plantings.each do |p| %li - = link_to "#{p.crop.system_name} in #{p.location}", p + = link_to "#{p.crop_system_name} in #{p.location}", p - if p.planted_at on = p.planted_at diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index cce9cf65f..b6266157b 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -43,4 +43,19 @@ describe Planting do Planting.first.should eq @planting2 end + context 'delegation' do + it 'system name' do + @planting.crop_system_name.should eq @planting.crop.system_name + end + it 'wikipedia url' do + @planting.crop_en_wikipedia_url.should eq @planting.crop.en_wikipedia_url + end + it 'default scientific name' do + @planting.crop_default_scientific_name.should eq @planting.crop.default_scientific_name + end + it 'plantings count' do + @planting.crop_plantings_count.should eq @planting.crop.plantings_count + end + end + end From c2a1e0853a890430d9e53f31247c9b302920895e Mon Sep 17 00:00:00 2001 From: Skud Date: Tue, 2 Apr 2013 15:28:27 +1100 Subject: [PATCH 22/23] Fixed bug where blank planting date didn't work --- app/models/planting.rb | 2 +- spec/models/planting_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index ab312f122..b28b11491 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -42,6 +42,6 @@ class Planting < ActiveRecord::Base end def planted_at_string=(str) - self.planted_at = Time.parse(str) + self.planted_at = str == '' ? nil : Time.parse(str) end end diff --git a/spec/models/planting_spec.rb b/spec/models/planting_spec.rb index 8dcffbdbd..00e4c7bb1 100644 --- a/spec/models/planting_spec.rb +++ b/spec/models/planting_spec.rb @@ -39,6 +39,11 @@ describe Planting do @planting.planted_at.should == Time.local(2013, 03, 01) end + it 'should accept blank dates' do + @planting.planted_at_string = '' + @planting.planted_at.should == nil + end + it "should output dates in ISO format" do @planting.planted_at = Time.local(2013, 03, 01) @planting.planted_at_string.should == "2013-03-01" From ae4b0cca2b5000d48f5e235648483aeb4c5ead0d Mon Sep 17 00:00:00 2001 From: Miles Gould Date: Tue, 2 Apr 2013 12:29:29 +0100 Subject: [PATCH 23/23] Remove extra route to notifications/new. --- config/routes.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index eddf2d230..413be78f9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,7 +1,5 @@ Growstuff::Application.routes.draw do - get "notifications/new" - devise_for :members, :controllers => { :registrations => "registrations" } resources :plantings