diff --git a/app/controllers/aspects_controller.rb b/app/controllers/aspects_controller.rb index c4fc158a2..9757df1a8 100644 --- a/app/controllers/aspects_controller.rb +++ b/app/controllers/aspects_controller.rb @@ -16,7 +16,8 @@ class AspectsController < ApplicationController helper_method :selected_people def index - @stream = AspectStream.new(current_user, params[:a_ids], + aspect_ids = (params[:a_ids] ? params[:a_ids] : []) + @stream = AspectStream.new(current_user, aspect_ids, :order => session[:sort_order], :max_time => params[:max_time]) @@ -82,8 +83,7 @@ class AspectsController < ApplicationController end def show - @aspect = current_user.aspects.where(:id => params[:id]).first - if @aspect + if @aspect = current_user.aspects.where(:id => params[:id]).first redirect_to aspects_path('a_ids[]' => @aspect.id) else redirect_to aspects_path diff --git a/lib/aspect_stream.rb b/lib/aspect_stream.rb index f90fde6fe..df8b50dc7 100644 --- a/lib/aspect_stream.rb +++ b/lib/aspect_stream.rb @@ -4,48 +4,64 @@ class AspectStream - attr_reader :aspects, :aspect_ids, :max_time, :order + attr_reader :max_time, :order # @param user [User] # @param aspect_ids [Array] Aspects this stream is responsible for - def initialize(user, aspect_ids, opts={}) + def initialize(user, inputted_aspect_ids, opts={}) @user = user - @aspects = user.aspects - @aspects = @aspects.where(:id => aspect_ids) if aspect_ids.present? - @aspect_ids = self.aspect_ids + @inputted_aspect_ids = inputted_aspect_ids - # ugly hack for now @max_time = opts[:max_time].to_i @order = opts[:order] end - # @return [Array] + # Filters aspects given the stream's aspect ids on initialization and the user. + # Will disclude aspects from inputted aspect ids if user is not associated with their + # target aspects. + # + # @return [ActiveRecord::Association] Filtered aspects given the stream's user + def aspects + @aspects ||= lambda do + a = @user.aspects + a = a.where(:id => @inputted_aspect_ids) if @inputted_aspect_ids.length > 0 + a + end.call + @aspects + end + + # Maps ids into an array from #aspects + # + # @return [Array] Aspect ids def aspect_ids - @aspect_ids ||= @aspects.map { |a| a.id } + @aspect_ids ||= aspects.map { |a| a.id } end # @return [ActiveRecord::Association] AR association of posts def posts + # NOTE(this should be something like User.aspect_post(@aspect_ids, {}) that calls visible_posts @posts ||= @user.visible_posts(:by_members_of => @aspect_ids, :type => ['StatusMessage','Reshare', 'ActivityStreams::Photo'], - :order => @order + ' DESC', + :order => "#{@order} DESC", :max_time => @max_time ).includes(:mentions => {:person => :profile}, :author => :profile) end # @return [ActiveRecord::Association] AR association of people within stream's given aspects def people + # NOTE(this should call a method in Person @people ||= Person.joins(:contacts => :aspect_memberships). where(:contacts => {:user_id => @user.id}, :aspect_memberships => {:aspect_id => @aspect_ids}). select("DISTINCT people.*").includes(:profile) end - # @note aspects.first is used for mobile. - # The :all is currently used for view switching logic + # @note aspects.first is used for mobile. NOTE(this is a hack and should be fixed) # @return [Aspect,Symbol] def aspect - for_all_aspects? ? :all : @aspects.first + if !for_all_aspects? || aspects.size == 1 + aspects.first + end end # Determine whether or not the stream is displaying across @@ -53,7 +69,7 @@ class AspectStream # # @return [Boolean] def for_all_aspects? - @aspect_ids.length == @aspects.length + @all_aspects ||= aspect_ids.length == @user.aspects.size end end diff --git a/spec/controllers/aspects_controller_spec.rb b/spec/controllers/aspects_controller_spec.rb index 61fa9a19b..584ffcf02 100644 --- a/spec/controllers/aspects_controller_spec.rb +++ b/spec/controllers/aspects_controller_spec.rb @@ -139,7 +139,7 @@ describe AspectsController do context 'with posts in multiple aspects' do before do - pending 'this spec needs to be moved into AspectStream spec' + pending 'this spec needs to be moved into AspectStream spec or visible_posts spec' @posts = [] 2.times do |n| user = Factory(:user) diff --git a/spec/lib/aspect_stream_spec.rb b/spec/lib/aspect_stream_spec.rb new file mode 100644 index 000000000..875396279 --- /dev/null +++ b/spec/lib/aspect_stream_spec.rb @@ -0,0 +1,106 @@ +require 'aspect_stream' + +describe AspectStream do + describe '#aspects' do + it 'queries the user given initialized aspect ids' do + alice = stub.as_null_object + stream = AspectStream.new(alice, [1,2,3]) + + alice.aspects.should_receive(:where) + stream.aspects + end + + it "returns all the user's aspects if no aspect ids are specified" do + alice = stub.as_null_object + stream = AspectStream.new(alice, []) + + alice.aspects.should_not_receive(:where) + stream.aspects + end + + it 'filters aspects given a user' do + alice = stub(:aspects => [stub(:id => 1)]) + alice.aspects.stub(:where).and_return(alice.aspects) + stream = AspectStream.new(alice, [1,2,3]) + + stream.aspects.should == alice.aspects + end + end + + describe '#aspect_ids' do + it 'maps ids from aspects' do + alice = stub.as_null_object + aspects = stub.as_null_object + + stream = AspectStream.new(alice, [1,2]) + + stream.should_receive(:aspects).and_return(aspects) + aspects.should_receive(:map) + stream.aspect_ids + end + end + + describe '#posts' do + before do + @alice = stub.as_null_object + end + + it 'calls visible posts for the given user' do + stream = AspectStream.new(@alice, [1,2]) + + @alice.should_receive(:visible_posts).and_return(stub.as_null_object) + stream.posts + end + + it 'respects ordering' do + stream = AspectStream.new(@alice, [1,2], :order => 'created_at') + @alice.should_receive(:visible_posts).with(hash_including(:order => 'created_at DESC')).and_return(stub.as_null_object) + stream.posts + end + + it 'respects max_time' do + stream = AspectStream.new(@alice, [1,2], :max_time => 123) + @alice.should_receive(:visible_posts).with(hash_including(:max_time => 123)).and_return(stub.as_null_object) + stream.posts + end + end + + describe '#people' do + it 'should call a method on person that doesnt exist yet' + end + + describe '#aspect' do + before do + alice = stub.as_null_object + @stream = AspectStream.new(alice, [1,2]) + end + + it "returns an aspect if the stream is not for all the user's aspects" do + @stream.stub(:for_all_aspects?).and_return(false) + @stream.aspect.should_not be_nil + end + + it "returns nothing if the stream is not for all the user's aspects" do + @stream.stub(:for_all_aspects?).and_return(true) + @stream.aspect.should be_nil + end + end + + describe 'for_all_aspects?' do + before do + alice = stub.as_null_object + alice.aspects.stub(:size).and_return(2) + @stream = AspectStream.new(alice, [1,2]) + end + + it "is true if the count of aspect_ids is equal to the size of the user's aspect count" do + @stream.aspect_ids.stub(:length).and_return(2) + @stream.should be_for_all_aspects + end + + it "is false if the count of aspect_ids is not equal to the size of the user's aspect count" do + @stream.aspect_ids.stub(:length).and_return(1) + @stream.should_not be_for_all_aspects + end + end +end