From e0eb76eb2a94c1e389b6fe5bfc68f0e7bf4a92ed Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Mon, 25 Dec 2017 00:13:24 +0100 Subject: [PATCH] Remove aspect_ids parameter from publisher closes #7683 --- Changelog.md | 1 + app/controllers/status_messages_controller.rb | 5 --- app/helpers/aspect_global_helper.rb | 4 +-- app/views/publisher/_publisher.html.haml | 2 -- .../status_messages/bookmarklet.mobile.haml | 2 +- app/views/status_messages/new.html.haml | 8 ++--- app/views/status_messages/new.mobile.haml | 2 +- lib/stream/aspect.rb | 15 ++++---- lib/stream/base.rb | 4 --- spec/lib/stream/aspect_spec.rb | 35 ++++++------------- 10 files changed, 23 insertions(+), 55 deletions(-) diff --git a/Changelog.md b/Changelog.md index 05a3fa7df..1096744f0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ ## Bug fixes * Ignore invalid `diaspora://` links [#7652](https://github.com/diaspora/diaspora/pull/7652) * Fix deformed avatar in hovercards [#7656](https://github.com/diaspora/diaspora/pull/7656) +* Fix default aspects on profile page and bookmarklet publisher [#7679](https://github.com/diaspora/diaspora/issues/7679) ## Features * Add birthday notifications [#7624](https://github.com/diaspora/diaspora/pull/7624) diff --git a/app/controllers/status_messages_controller.rb b/app/controllers/status_messages_controller.rb index 77a4bdf38..20d4c1c97 100644 --- a/app/controllers/status_messages_controller.rb +++ b/app/controllers/status_messages_controller.rb @@ -20,8 +20,6 @@ class StatusMessagesController < ApplicationController @contact = current_user.contact_for(@person) if @contact @aspects_with_person = @contact.aspects.load - @aspect_ids = @aspects_with_person.map(&:id) - gon.aspect_ids = @aspect_ids render layout: nil else @aspects_with_person = [] @@ -29,8 +27,6 @@ class StatusMessagesController < ApplicationController elsif request.format == :mobile @aspect = :all @aspects = current_user.aspects.load - @aspect_ids = @aspects.map(&:id) - gon.aspect_ids = @aspect_ids else redirect_to stream_path end @@ -38,7 +34,6 @@ class StatusMessagesController < ApplicationController def bookmarklet @aspects = current_user.aspects - @aspect_ids = current_user.aspect_ids gon.preloads[:bookmarklet] = { content: params[:content], diff --git a/app/helpers/aspect_global_helper.rb b/app/helpers/aspect_global_helper.rb index 49ce5bab4..70ff11deb 100644 --- a/app/helpers/aspect_global_helper.rb +++ b/app/helpers/aspect_global_helper.rb @@ -17,14 +17,12 @@ module AspectGlobalHelper if stream aspects = stream.aspects aspect = stream.aspect - aspect_ids = stream.aspect_ids elsif current_user aspects = current_user.post_default_aspects aspect = aspects.first - aspect_ids = current_user.aspect_ids else return {} end - {selected_aspects: aspects, aspect: aspect, aspect_ids: aspect_ids} + {selected_aspects: aspects, aspect: aspect} end end diff --git a/app/views/publisher/_publisher.html.haml b/app/views/publisher/_publisher.html.haml index 79b5636cd..808a79060 100644 --- a/app/views/publisher/_publisher.html.haml +++ b/app/views/publisher/_publisher.html.haml @@ -62,6 +62,4 @@ data: {toggle: "modal", target: "#publicExplainModal"}} %i.entypo-cog - = link_to "", contacts_path(aspect_ids: aspect_ids), class: "selected_contacts_link hidden" - = render "shared/public_explain" diff --git a/app/views/status_messages/bookmarklet.mobile.haml b/app/views/status_messages/bookmarklet.mobile.haml index 0dafbc991..ca5428ad4 100644 --- a/app/views/status_messages/bookmarklet.mobile.haml +++ b/app/views/status_messages/bookmarklet.mobile.haml @@ -2,6 +2,6 @@ -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. -= render :partial => 'publisher/publisher', :locals => { :aspect => :profile, :selected_aspects => @aspects, :aspect_ids => @aspect_ids } += render partial: "publisher/publisher", locals: {aspect: :profile, selected_aspects: @aspects} = javascript_include_tag "mobile/bookmarklet" diff --git a/app/views/status_messages/new.html.haml b/app/views/status_messages/new.html.haml index 5eaf54d9d..b21eb6172 100644 --- a/app/views/status_messages/new.html.haml +++ b/app/views/status_messages/new.html.haml @@ -1,6 +1,2 @@ -= render :partial => 'publisher/publisher', - :locals => { :aspect => @aspect, - :aspect_ids => @aspect_ids, - :selected_aspects => @aspects_with_person, - :person => @person } - += render partial: "publisher/publisher", + locals: {aspect: @aspect, selected_aspects: @aspects_with_person, person: @person} diff --git a/app/views/status_messages/new.mobile.haml b/app/views/status_messages/new.mobile.haml index c12ddff48..d7f8ce025 100644 --- a/app/views/status_messages/new.mobile.haml +++ b/app/views/status_messages/new.mobile.haml @@ -2,4 +2,4 @@ -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. -= render :partial => 'publisher/publisher', :locals => {:aspect => @aspects.first, :aspect_ids => @aspect_ids, :selected_aspects => @aspects} += render partial: "publisher/publisher", locals: {aspect: @aspects.first, selected_aspects: @aspects} diff --git a/lib/stream/aspect.rb b/lib/stream/aspect.rb index 6800310b0..af6972775 100644 --- a/lib/stream/aspect.rb +++ b/lib/stream/aspect.rb @@ -30,13 +30,6 @@ class Stream::Aspect < Stream::Base end.call end - # Maps ids into an array from #aspects - # - # @return [Array] Aspect ids - def aspect_ids - @aspect_ids ||= aspects.map { |a| a.id } - end - # @return [ActiveRecord::Association] AR association of posts def posts # NOTE(this should be something like Post.all_for_stream(@user, aspect_ids, {}) that calls visible_shareables @@ -84,7 +77,7 @@ class Stream::Aspect < Stream::Base # # @return [Boolean] def for_all_aspects? - @all_aspects ||= aspect_ids.length == user.aspects.size + @all_aspects ||= aspects.size == user.aspects.size end # This is perfomance optimization, as everyone in your aspect stream you have @@ -95,4 +88,10 @@ class Stream::Aspect < Stream::Base def can_comment?(post) true end + + private + + def aspect_ids + @aspect_ids ||= aspects.map(&:id) + end end diff --git a/lib/stream/base.rb b/lib/stream/base.rb index 9e45666e3..30ac2f92c 100644 --- a/lib/stream/base.rb +++ b/lib/stream/base.rb @@ -67,10 +67,6 @@ class Stream::Base aspects.first end - def aspect_ids - aspects.map {|x| x.try(:id) } - end - def max_time=(time_string) @max_time = Time.at(time_string.to_i) unless time_string.blank? @max_time ||= (Time.now + 1) diff --git a/spec/lib/stream/aspect_spec.rb b/spec/lib/stream/aspect_spec.rb index 21f8ec7b7..54507a9bc 100644 --- a/spec/lib/stream/aspect_spec.rb +++ b/spec/lib/stream/aspect_spec.rb @@ -31,19 +31,6 @@ describe Stream::Aspect do end end - describe '#aspect_ids' do - it 'maps ids from aspects' do - alice = double.as_null_object - aspects = double.as_null_object - - stream = Stream::Aspect.new(alice, [1,2]) - - expect(stream).to receive(:aspects).and_return(aspects) - expect(aspects).to receive(:map) - stream.aspect_ids - end - end - describe '#posts' do before do @alice = double.as_null_object @@ -83,16 +70,14 @@ describe Stream::Aspect do end end - describe '#people' do - it 'should call Person.all_from_aspects' do - class Person ; end - + describe "#people" do + it "should call Person.all_from_aspects" do alice = double.as_null_object - aspect_ids = [1,2,3] + aspect_ids = [1, 2, 3] stream = Stream::Aspect.new(alice, []) allow(stream).to receive(:aspect_ids).and_return(aspect_ids) - expect(Person).to receive(:unique_from_aspects).with(stream.aspect_ids, alice).and_return(double(:includes => :profile)) + expect(Person).to receive(:unique_from_aspects).with(aspect_ids, alice).and_return(double(includes: :profile)) stream.people end end @@ -114,20 +99,20 @@ describe Stream::Aspect do end end - describe 'for_all_aspects?' do + describe "for_all_aspects?" do before do alice = double.as_null_object allow(alice.aspects).to receive(:size).and_return(2) - @stream = Stream::Aspect.new(alice, [1,2]) + @stream = Stream::Aspect.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 - allow(@stream.aspect_ids).to receive(:length).and_return(2) + it "is true if the count of aspects is equal to the size of the user's aspect count" do + allow(@stream).to receive(:aspects).and_return(double(size: 2)) expect(@stream).to 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 - allow(@stream.aspect_ids).to receive(:length).and_return(1) + it "is false if the count of aspects is not equal to the size of the user's aspect count" do + allow(@stream).to receive(:aspects).and_return(double(size: 1)) expect(@stream).not_to be_for_all_aspects end end