From 86e374b8ceb7f44243c87fc67b7445741778ce17 Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg <maxwell@joindiaspora.com> Date: Tue, 9 Aug 2011 16:54:16 -0700 Subject: [PATCH] add hovercards back to mentions, reorganized some helpers, made getting started view mobile accessible --- app/controllers/profiles_controller.rb | 2 +- app/controllers/vanna_controller.rb | 3 + app/helpers/application_helper.rb | 51 --------------- app/helpers/notifications_helper.rb | 3 + app/helpers/people_helper.rb | 41 +++++++++++- app/helpers/users_helper.rb | 13 ++++ app/models/status_message.rb | 3 +- ...started.html.haml => getting_started.haml} | 0 spec/controllers/users_controller_spec.rb | 15 +++++ spec/helpers/application_helper_spec.rb | 53 +--------------- spec/helpers/people_helper_spec.rb | 63 +++++++++++++++++++ spec/models/status_message_spec.rb | 9 +-- 12 files changed, 147 insertions(+), 109 deletions(-) create mode 100644 app/helpers/users_helper.rb rename app/views/users/{getting_started.html.haml => getting_started.haml} (100%) create mode 100644 spec/helpers/people_helper_spec.rb diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 51e4f2dddf..cd3530c129 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -37,7 +37,7 @@ class ProfilesController < ApplicationController if current_user.update_profile params[:profile] flash[:notice] = I18n.t 'profiles.update.updated' if current_user.getting_started? - redirect_to getting_started_path + redirect_to getting_started_path, :notice => flash[:notice] else redirect_to edit_profile_path end diff --git a/app/controllers/vanna_controller.rb b/app/controllers/vanna_controller.rb index 6883b16dec..ce0728fda0 100644 --- a/app/controllers/vanna_controller.rb +++ b/app/controllers/vanna_controller.rb @@ -5,6 +5,9 @@ class VannaController < Vanna::Base include Devise::Controllers::Helpers include AspectGlobalHelper + include PeopleHelper + include UsersHelper + helper :layout helper_method :current_user helper_method :all_aspects diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 7aa4eb30ea..12951f2fd0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -21,67 +21,16 @@ module ApplicationHelper object.attributes.keys end - def mine?(post) - current_user.owns? post - end def type_partial(post) class_name = post.class.name.to_s.underscore "#{class_name.pluralize}/#{class_name}" end - def profile_photo(person) - person_image_link(person, :size => :thumb_large, :to => :photos) - end - - def owner_image_tag(size=nil) - person_image_tag(current_user.person, size) - end - - def owner_image_link - person_image_link(current_user.person) - end - - def person_image_tag(person, size=nil) - size ||= :thumb_small - "<img alt=\"#{h(person.name)}\" class=\"avatar\" data-person_id=\"#{person.id}\" src=\"#{person.profile.image_url(size)}\" title=\"#{h(person.name)} (#{h(person.diaspora_handle)})\">".html_safe - end - - def person_link(person, opts={}) - opts[:class] ||= "" - opts[:class] << " self" if defined?(user_signed_in?) && user_signed_in? && current_user.person == person - remote_or_hovercard_link = "/people/#{person.id}".html_safe - if person.local? - "<a data-hovercard='#{remote_or_hovercard_link}' href='/u/#{person.diaspora_handle.split('@')[0]}' class='#{opts[:class]}'> - #{h(person.name)} - </a>".html_safe - else - "<a href='#{remote_or_hovercard_link}' data-hovercard='#{remote_or_hovercard_link}' class='#{opts[:class]}' > - #{h(person.name)} - </a>".html_safe - end - end - def hard_link(string, path) link_to string, path, :rel => 'external' end - def person_image_link(person, opts={}) - return "" if person.nil? || person.profile.nil? - if opts[:to] == :photos - link_to person_image_tag(person, opts[:size]), person_photos_path(person) - else - if person.local? - "<a href='/u/#{person.diaspora_handle.split('@')[0]}' class='#{opts[:class]}'> - #{person_image_tag(person, opts[:size])} - </a>".html_safe - else - "<a href='/people/#{person.id}'> - #{person_image_tag(person, opts[:size])} - </a>".html_safe - end - end - end def post_yield_tag(post) (':' + post.id.to_s).to_sym diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 9d7a30f170..bd288fee86 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -2,7 +2,10 @@ module NotificationsHelper include ERB::Util include ActionView::Helpers::TranslationHelper include ActionView::Helpers::UrlHelper + include PeopleHelper + include UsersHelper include ApplicationHelper + def object_link(note, actors) target_type = note.popup_translation_key actors_count = note.actors.count diff --git a/app/helpers/people_helper.rb b/app/helpers/people_helper.rb index b149abca86..c4b971e424 100644 --- a/app/helpers/people_helper.rb +++ b/app/helpers/people_helper.rb @@ -3,7 +3,7 @@ # the COPYRIGHT file. module PeopleHelper - + include ERB::Util def request_partial single_aspect_form if single_aspect_form 'requests/new_request_with_aspect_to_person' @@ -27,4 +27,43 @@ module PeopleHelper I18n.l bday, :format => I18n.t('date.formats.birthday_with_year') end end + + def person_link(person, opts={}) + opts[:class] ||= "" + opts[:class] << " self" if defined?(user_signed_in?) && user_signed_in? && current_user.person == person + remote_or_hovercard_link = "/people/#{person.id}".html_safe + if person.local? + "<a data-hovercard='#{remote_or_hovercard_link}' href='/u/#{person.diaspora_handle.split('@')[0]}' class='#{opts[:class]}'> + #{h(person.name)} + </a>".html_safe + else + "<a href='#{remote_or_hovercard_link}' data-hovercard='#{remote_or_hovercard_link}' class='#{opts[:class]}' > + #{h(person.name)} + </a>".html_safe + end + end + + + def person_image_tag(person, size=nil) + size ||= :thumb_small + "<img alt=\"#{h(person.name)}\" class=\"avatar\" data-person_id=\"#{person.id}\" src=\"#{person.profile.image_url(size)}\" title=\"#{h(person.name)} (#{h(person.diaspora_handle)})\">".html_safe + end + + def person_image_link(person, opts={}) + return "" if person.nil? || person.profile.nil? + if opts[:to] == :photos + link_to person_image_tag(person, opts[:size]), person_photos_path(person) + else + if person.local? + "<a href='/u/#{person.diaspora_handle.split('@')[0]}' class='#{opts[:class]}'> + #{person_image_tag(person, opts[:size])} + </a>".html_safe + else + "<a href='/people/#{person.id}'> + #{person_image_tag(person, opts[:size])} + </a>".html_safe + end + end + end + end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb new file mode 100644 index 0000000000..d791bf86db --- /dev/null +++ b/app/helpers/users_helper.rb @@ -0,0 +1,13 @@ +module UsersHelper + def owner_image_tag(size=nil) + person_image_tag(current_user.person, size) + end + + def owner_image_link + person_image_link(current_user.person) + end + + def mine?(post) + current_user.owns? post + end +end diff --git a/app/models/status_message.rb b/app/models/status_message.rb index fd460f245d..58ed733bb6 100644 --- a/app/models/status_message.rb +++ b/app/models/status_message.rb @@ -9,6 +9,7 @@ class StatusMessage < Post include YoutubeTitles require File.join(Rails.root, 'lib/youtube_titles') include ActionView::Helpers::TextHelper + include PeopleHelper acts_as_taggable_on :tags extract_tags_from :raw_message @@ -55,7 +56,7 @@ class StatusMessage < Post if opts[:plain_text] person ? ERB::Util.h(person.name) : ERB::Util.h($~[1]) else - person ? "<a href=\"/people/#{person.id}\" class=\"mention hovercardable\">@#{ERB::Util.h(person.name)}</a>" : ERB::Util.h($~[1]) + person ? person_link(person, :class => 'mention hovercardable') : ERB::Util.h($~[1]) end end form_message diff --git a/app/views/users/getting_started.html.haml b/app/views/users/getting_started.haml similarity index 100% rename from app/views/users/getting_started.html.haml rename to app/views/users/getting_started.haml diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 66a445a3a7..71f1871565 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -205,4 +205,19 @@ describe UsersController do request.flash[:notice].should be_blank end end + + describe 'getting_started' do + it 'does not fail miserably' do + get :getting_started + response.should be_success + + end + + it 'does not fail miserably on mobile' do + get :getting_started, :format => :mobile + response.should be_success + + end + end end + diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 2c393e36d9..824fc044ac 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,4 +1,4 @@ -# Copyright (c) 2010, Diaspora Inc. This file is +# Copyright (c) 2011, Diaspora Inc. This file is # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. @@ -10,56 +10,7 @@ describe ApplicationHelper do @person = Factory.create(:person) end - describe "#person_image_link" do - it "returns an empty string if person is nil" do - person_image_link(nil).should == "" - end - it "returns a link containing the person's photo" do - person_image_link(@person).should include(@person.profile.image_url) - end - it "returns a link to the person's profile" do - person_image_link(@person).should include(person_path(@person)) - end - end - - describe "#person_image_tag" do - it "should not allow basic XSS/HTML" do - @person.profile.first_name = "I'm <h1>Evil" - @person.profile.last_name = "I'm <h1>Evil" - person_image_tag(@person).should_not include("<h1>") - end - end - - describe '#person_link' do - before do - @person = Factory(:person) - end - - it 'includes the name of the person if they have a first name' do - person_link(@person).should include @person.profile.first_name - end - - it 'uses diaspora handle if the person has no first or last name' do - @person.profile.first_name = nil - @person.profile.last_name = nil - - person_link(@person).should include @person.diaspora_handle - end - - it 'uses diaspora handle if first name and first name are rails#blank?' do - @person.profile.first_name = " " - @person.profile.last_name = " " - - person_link(@person).should include @person.diaspora_handle - end - - it "should not allow basic XSS/HTML" do - @person.profile.first_name = "I'm <h1>Evil" - @person.profile.last_name = "I'm <h1>Evil" - person_link(@person).should_not include("<h1>") - end - end - + describe "#contacts_link" do before do def current_user diff --git a/spec/helpers/people_helper_spec.rb b/spec/helpers/people_helper_spec.rb new file mode 100644 index 0000000000..320dcddc53 --- /dev/null +++ b/spec/helpers/people_helper_spec.rb @@ -0,0 +1,63 @@ +# Copyright (c) 2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require 'spec_helper' + +describe PeopleHelper do + before do + @user = alice + @person = Factory.create(:person) + end + describe "#person_image_link" do + it "returns an empty string if person is nil" do + person_image_link(nil).should == "" + end + it "returns a link containing the person's photo" do + person_image_link(@person).should include(@person.profile.image_url) + end + it "returns a link to the person's profile" do + person_image_link(@person).should include(person_path(@person)) + end + end + + describe "#person_image_tag" do + it "should not allow basic XSS/HTML" do + @person.profile.first_name = "I'm <h1>Evil" + @person.profile.last_name = "I'm <h1>Evil" + person_image_tag(@person).should_not include("<h1>") + end + end + + describe '#person_link' do + before do + @person = Factory(:person) + end + + it 'includes the name of the person if they have a first name' do + person_link(@person).should include @person.profile.first_name + end + + it 'uses diaspora handle if the person has no first or last name' do + @person.profile.first_name = nil + @person.profile.last_name = nil + + person_link(@person).should include @person.diaspora_handle + end + + it 'uses diaspora handle if first name and first name are rails#blank?' do + @person.profile.first_name = " " + @person.profile.last_name = " " + + person_link(@person).should include @person.diaspora_handle + end + + it "should not allow basic XSS/HTML" do + @person.profile.first_name = "I'm <h1>Evil" + @person.profile.last_name = "I'm <h1>Evil" + person_link(@person).should_not include("<h1>") + end + end + +end + diff --git a/spec/models/status_message_spec.rb b/spec/models/status_message_spec.rb index d761815437..b60cf6f9b9 100644 --- a/spec/models/status_message_spec.rb +++ b/spec/models/status_message_spec.rb @@ -6,6 +6,7 @@ require 'spec_helper' describe StatusMessage do include ActionView::Helpers::UrlHelper + include PeopleHelper include Rails.application.routes.url_helpers def controller mock() @@ -86,10 +87,10 @@ STR describe '#format_mentions' do it 'adds the links in the formated message text' do - @sm.format_mentions(@sm.raw_message).should == <<-STR -#{link_to('@' << @people[0].name, person_path(@people[0]), :class => 'mention hovercardable')} can mention people like Raphael #{link_to('@' << @people[1].name, person_path(@people[1]), :class => 'mention hovercardable')} -can mention people like Raphaellike Raphael #{link_to('@' << @people[2].name, person_path(@people[2]), :class => 'mention hovercardable')} can mention people like Raph -STR + message = @sm.format_mentions(@sm.raw_message) + message.should include(person_link(@people[0], :class => 'mention hovercardable')) + message.should include(person_link(@people[1], :class => 'mention hovercardable')) + message.should include(person_link(@people[2], :class => 'mention hovercardable')) end context 'with :plain_text option' do -- GitLab