From 893d92ac722dc9ecd25fd5214d8a1c6719081479 Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 11:39:03 +1100 Subject: [PATCH 1/8] Added 'Settings' to top nav --- app/controllers/members_controller.rb | 3 +++ app/views/layouts/_header.html.haml | 10 ++++++++-- spec/views/layouts/application_spec.rb | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 40a50f161..d80e688cb 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -25,4 +25,7 @@ class MembersController < ApplicationController end end + def edit + end + end diff --git a/app/views/layouts/_header.html.haml b/app/views/layouts/_header.html.haml index 6a931c173..497b228c4 100644 --- a/app/views/layouts/_header.html.haml +++ b/app/views/layouts/_header.html.haml @@ -17,8 +17,14 @@ %li= link_to("Plant something", new_planting_path) %li.divider-vertical - if member_signed_in? - %li= link_to current_member.login_name, member_path(current_member) - %li= link_to "Log out", destroy_member_session_path, :method => :delete + %li.dropdown< + %a.dropdown-toggle{'data-toggle' => 'dropdown', :href => member_path(current_member)} + = current_member.login_name + %b.caret + %ul.dropdown-menu + %li= link_to "Profile", member_path(current_member) + %li= link_to "Settings", url_for(:controller => 'members', :action => 'edit', :id => current_member) + %li= link_to "Log out", destroy_member_session_path, :method => :delete - else %li.pull-right= link_to 'Sign up', new_member_registration_path %li.pull-right= link_to 'Log in', new_member_session_path diff --git a/spec/views/layouts/application_spec.rb b/spec/views/layouts/application_spec.rb index eb70be5c2..02ca091a6 100644 --- a/spec/views/layouts/application_spec.rb +++ b/spec/views/layouts/application_spec.rb @@ -41,6 +41,14 @@ describe 'layouts/application.html.haml', :type => "view" do rendered.should contain 'Plant something' end + it "should show member's name" do + assert_select("a[href=/members/#{@member.login_name}]", "Profile") + end + + it "should show settings link" do + assert_select "a[href=/members/#{@member.login_name}/edit]", "Settings" + end + it 'should show logout link' do rendered.should contain 'Log out' end From fb37e1abaa3537aef45597dad4dbfd81e282151f Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 11:41:08 +1100 Subject: [PATCH 2/8] fixed a weird variable expansion --- spec/views/layouts/_footer_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/views/layouts/_footer_spec.rb b/spec/views/layouts/_footer_spec.rb index 551dc46f2..fa61ea8f1 100644 --- a/spec/views/layouts/_footer_spec.rb +++ b/spec/views/layouts/_footer_spec.rb @@ -8,8 +8,8 @@ describe 'layouts/_footer.html.haml', :type => "view" do it 'should have links in footer' do rendered.should contain 'About' - assert_select("a[href=#{'/policy/tos'}]", 'Terms of Service') - assert_select("a[href=#{'/policy/community'}]", 'Community Guidelines') + assert_select("a[href=/policy/tos]", 'Terms of Service') + assert_select("a[href=/policy/community]", 'Community Guidelines') rendered.should contain 'License' rendered.should contain 'Github' rendered.should contain 'Mailing list' From e5d380729ed18bd7034ae51ba530dcd103d1d5ec Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 12:46:45 +1100 Subject: [PATCH 3/8] we more or less have a settings page. doesn't WORK though. --- app/controllers/members_controller.rb | 3 --- app/views/devise/registrations/edit.html.haml | 8 ------- app/views/layouts/_header.html.haml | 2 +- spec/controllers/member_controller_spec.rb | 21 ++++++++++--------- spec/controllers/posts_controller_spec.rb | 2 +- spec/views/layouts/application_spec.rb | 2 +- 6 files changed, 14 insertions(+), 24 deletions(-) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index d80e688cb..40a50f161 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -25,7 +25,4 @@ class MembersController < ApplicationController end end - def edit - end - end diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 8c1ee88de..d784a2255 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -27,11 +27,3 @@ %div = f.submit "Update" - -%h3 Cancel my account - -%p - Unhappy? - = link_to "Cancel my account", registration_path(resource_name), :data => { :confirm => "Are you sure?" }, :method => :delete. - -= link_to "Back", :back diff --git a/app/views/layouts/_header.html.haml b/app/views/layouts/_header.html.haml index 497b228c4..44a9e3434 100644 --- a/app/views/layouts/_header.html.haml +++ b/app/views/layouts/_header.html.haml @@ -23,7 +23,7 @@ %b.caret %ul.dropdown-menu %li= link_to "Profile", member_path(current_member) - %li= link_to "Settings", url_for(:controller => 'members', :action => 'edit', :id => current_member) + %li= link_to "Settings", edit_member_registration_path %li= link_to "Log out", destroy_member_session_path, :method => :delete - else %li.pull-right= link_to 'Sign up', new_member_registration_path diff --git a/spec/controllers/member_controller_spec.rb b/spec/controllers/member_controller_spec.rb index bc928fbe4..7cfa48c6a 100644 --- a/spec/controllers/member_controller_spec.rb +++ b/spec/controllers/member_controller_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' describe MembersController do before :each do - @member1 = FactoryGirl.create(:member) - @posts = [ FactoryGirl.create(:post, :author => @member1) ] + @member = FactoryGirl.create(:member) + @posts = [ FactoryGirl.create(:post, :author => @member) ] end describe "GET index" do - it "assigns only confirmed members as @member1s" do + it "assigns only confirmed members as @members" do get :index, {} - assigns(:members).should eq([@member1]) + assigns(:members).should eq([@member]) end end @@ -22,18 +22,18 @@ describe MembersController do end describe "GET show" do - it "assigns the requested member as @member1" do - get :show, {:id => @member1.id} - assigns(:member).should eq(@member1) + it "assigns the requested member as @member" do + get :show, {:id => @member.id} + assigns(:member).should eq(@member) end it "does NOT provide JSON for member profile" do - get :show, { :id => @member1.id , :format => 'json' } + get :show, { :id => @member.id , :format => 'json' } response.should_not be_success end it "assigns @posts with the member's posts" do - get :show, {:id => @member1.id} + get :show, {:id => @member.id} assigns(:posts).should eq(@posts) end @@ -50,10 +50,11 @@ describe MembersController do describe "GET member's RSS feed" do it "returns an RSS feed" do - get :show, { :id => @member1.to_param, :format => "rss" } + get :show, { :id => @member.to_param, :format => "rss" } response.should be_success response.should render_template("members/show") response.content_type.should eq("application/rss+xml") end end + end diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb index 16dc3ddaf..c8ad395c6 100644 --- a/spec/controllers/posts_controller_spec.rb +++ b/spec/controllers/posts_controller_spec.rb @@ -86,7 +86,7 @@ describe PostsController do end end - describe "PUT post" do + describe "PUT update" do describe "with valid params" do it "updates the requested post" do post = Post.create! valid_attributes diff --git a/spec/views/layouts/application_spec.rb b/spec/views/layouts/application_spec.rb index 02ca091a6..9cfeee0e5 100644 --- a/spec/views/layouts/application_spec.rb +++ b/spec/views/layouts/application_spec.rb @@ -46,7 +46,7 @@ describe 'layouts/application.html.haml', :type => "view" do end it "should show settings link" do - assert_select "a[href=/members/#{@member.login_name}/edit]", "Settings" + assert_select "a[href=/members/edit]", "Settings" end it 'should show logout link' do From dfac54d936c63548df6ebba4ebc9f06af52300e0 Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 15:31:14 +1100 Subject: [PATCH 4/8] Added hidden TOS field to edit settings page so it works --- app/views/devise/registrations/edit.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index d784a2255..a4addaf7b 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -27,3 +27,5 @@ %div = f.submit "Update" + + =f.hidden_field(:tos_agreement, :value => true) From 9b458070e01387b6859a8aa1ab63f9ea6f8c94e1 Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 15:54:43 +1100 Subject: [PATCH 5/8] Settings form now works (and is prettified) Had to add some Devise crud based on advice on their wiki, otherwise you couldn't change your settings without changing your password. Also had to make a hidden field for tos_agreement. I don't really love it but nobody seems to be able to recommend a better way. --- app/controllers/registrations_controller.rb | 24 ++++++++++ app/views/devise/registrations/edit.html.haml | 47 ++++++++++--------- config/routes.rb | 2 +- 3 files changed, 50 insertions(+), 23 deletions(-) create mode 100644 app/controllers/registrations_controller.rb diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb new file mode 100644 index 000000000..566ecb94c --- /dev/null +++ b/app/controllers/registrations_controller.rb @@ -0,0 +1,24 @@ +# we need this subclass so that Devise doesn't force people to change their +# password every time they want to edit their settings. Code copied from +# https://github.com/plataformatec/devise/wiki/How-To:-Allow-users-to-edit-their-account-without-providing-a-password + +class RegistrationsController < Devise::RegistrationsController + def update + # required for settings form to submit when password is left blank + if params[:member][:password].blank? + params[:member].delete("password") + params[:member].delete("password_confirmation") + params[:member].delete("current_password") + end + + @member = Member.find(current_member.id) + if @member.update_attributes(params[:member]) + set_flash_message :notice, :updated + # Sign in the member bypassing validation in case his password changed + sign_in @member, :bypass => true + redirect_to after_update_path_for(@member) + else + render "edit" + end + end +end diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index a4addaf7b..7a6f1943c 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -1,31 +1,34 @@ -- content_for :title, "Edit " + resource_name.to_s.humanize +- content_for :title, "Settings for #{current_member.login_name}" -= form_for(resource, :as => resource_name, :url => registration_path(resource_name), :html => { :method => :put }) do |f| += form_for(resource, :as => resource_name, :url => registration_path(resource_name), :html => { :method => :put, :class => 'form-horizontal' }) do |f| = devise_error_messages! - %div - = f.label :email - %br - = f.email_field :email + %h2 Email address - %div - = f.label :password - %i (leave blank if you don't want to change it) - %br - = f.password_field :password, :autocomplete => "off" + .control-group + = f.label :email, :class => 'control-label' + .controls + = f.email_field :email + %span.help-inline If you change your email address you will have to reconfirm - %div - = f.label :password_confirmation - %br - = f.password_field :password_confirmation + %h2 Change password - %div - = f.label :current_password - %i (we need your current password to confirm your changes) - %br - = f.password_field :current_password + .control-group + = f.label :password, :class => 'control-label' + .controls + = f.password_field :password, :autocomplete => "off" + %span.help-inline Leave blank if you don't want to change your password - %div - = f.submit "Update" + .control-group + = f.label :password_confirmation, :class => 'control-label' + .controls= f.password_field :password_confirmation + + .control-group + = f.label :current_password, :class => 'control-label' + .controls + = f.password_field :current_password + + .form-actions + = f.submit "Update", :class => 'btn' =f.hidden_field(:tos_agreement, :value => true) diff --git a/config/routes.rb b/config/routes.rb index fe39180dd..9dd4b7ad0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,5 @@ Growstuff::Application.routes.draw do - devise_for :members + devise_for :members, :controllers => { :registrations => "registrations" } resources :plantings resources :gardens From 1dbed949f616eadf1a7bd4dee4c40806079e4d32 Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 15:56:51 +1100 Subject: [PATCH 6/8] s/Log in/Sign in/ -- be consistent with devise --- app/views/layouts/_header.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/_header.html.haml b/app/views/layouts/_header.html.haml index 44a9e3434..d1bb8d7e0 100644 --- a/app/views/layouts/_header.html.haml +++ b/app/views/layouts/_header.html.haml @@ -24,7 +24,7 @@ %ul.dropdown-menu %li= link_to "Profile", member_path(current_member) %li= link_to "Settings", edit_member_registration_path - %li= link_to "Log out", destroy_member_session_path, :method => :delete + %li= link_to "Sign out", destroy_member_session_path, :method => :delete - else %li.pull-right= link_to 'Sign up', new_member_registration_path - %li.pull-right= link_to 'Log in', new_member_session_path + %li.pull-right= link_to 'Sign in', new_member_session_path From a50bee4828516eb937537ef285ec49ec54193822 Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 16:00:47 +1100 Subject: [PATCH 7/8] oops, broke tests with the change to 'Sign in' --- spec/views/layouts/application_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/views/layouts/application_spec.rb b/spec/views/layouts/application_spec.rb index 9cfeee0e5..a29e668c5 100644 --- a/spec/views/layouts/application_spec.rb +++ b/spec/views/layouts/application_spec.rb @@ -15,7 +15,7 @@ describe 'layouts/application.html.haml', :type => "view" do it 'should have signup/login links' do rendered.should contain 'Sign up' - rendered.should contain 'Log in' + rendered.should contain 'Sign in' end end @@ -50,7 +50,7 @@ describe 'layouts/application.html.haml', :type => "view" do end it 'should show logout link' do - rendered.should contain 'Log out' + rendered.should contain 'Sign out' end end From 4f64946de22c7be1dc4b2c5f79dfba228f7ac8ad Mon Sep 17 00:00:00 2001 From: Skud Date: Wed, 6 Feb 2013 16:10:43 +1100 Subject: [PATCH 8/8] Wrote some very basic tests for the improved settings page --- spec/views/devise/registrations/edit_spec.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/spec/views/devise/registrations/edit_spec.rb b/spec/views/devise/registrations/edit_spec.rb index 0ccc2a80f..25bb6d4c5 100644 --- a/spec/views/devise/registrations/edit_spec.rb +++ b/spec/views/devise/registrations/edit_spec.rb @@ -1,10 +1,13 @@ require 'spec_helper' -describe 'devise/registrations/edit.html.erb', :type => "view" do +describe 'devise/registrations/edit.html.haml', :type => "view" do context "logged in" do before(:each) do - @view.stub(:resource).and_return(Member.new) + controller.stub(:current_user) { nil } + @member = FactoryGirl.create(:member) + controller.stub(:current_member) { @member } + @view.stub(:resource).and_return(@member) @view.stub(:resource_name).and_return("member") @view.stub(:resource_class).and_return(Member) @view.stub(:devise_mapping).and_return(Devise.mappings[:member]) @@ -13,7 +16,15 @@ describe 'devise/registrations/edit.html.erb', :type => "view" do it 'should have some fields' do rendered.should contain 'Email' - rendered.should contain 'Cancel my account' end + + it 'should have an email section' do + assert_select "h2", "Email address" + end + + it 'should have a password section' do + assert_select "h2", "Change password" + end + end end