Always show public photos

closes #6398

app/assets/templates/profile_header_tpl.jst.hbs

@@ -20,85 +20,81 @@
     {{/if}}
   </h2>
 
-  {{#if loggedIn}}
-    {{#if has_tags}}
+  {{#if has_tags}}
+    <div class="description">
+      <i class="entypo tag"></i>
+      {{fmtTags profile.tags}}
+    </div>
+  {{else}}
+    {{#if is_own_profile}}
       <div class="description">
-        <i class="entypo tag"></i>
-        {{fmtTags profile.tags}}
+        <i>{{t 'profile.you_have_no_tags'}}</i>
+        <span class="add_tags">
+          <a href="{{urlTo 'editProfile'}}">{{t 'profile.add_some'}}</a>
+        </span>
       </div>
-    {{else}}
-      {{#if is_own_profile}}
-        <div class="description">
-          <i>{{t 'profile.you_have_no_tags'}}</i>
-          <span class="add_tags">
-            <a href="{{urlTo 'editProfile'}}">{{t 'profile.add_some'}}</a>
-          </span>
-        </div>
-      {{/if}}
     {{/if}}
   {{/if}}
 </div>
 
-{{#if loggedIn}}
-  <div id="profile_horizontal_bar">
-    {{#if show_profile_btns}}
-      <div id="profile_buttons" class="pull-right">
-        {{#if is_receiving}}
-          {{!-- create status message with mention --}}
-          <span class="profile_button">
-            <span id="mention_button" class="profile-header-icon" title="{{t 'people.mention'}}" data-placement="bottom" data-toggle="modal" data-target="#mentionModal">@</span>
-          </span>
-        {{/if}}
+<div id="profile_horizontal_bar">
+  {{#if show_profile_btns}}
+    <div id="profile_buttons" class="pull-right">
+      {{#if is_receiving}}
+        {{!-- create status message with mention --}}
+        <span class="profile_button">
+          <span id="mention_button" class="profile-header-icon" title="{{t 'people.mention'}}" data-placement="bottom" data-toggle="modal" data-target="#mentionModal">@</span>
+        </span>
+      {{/if}}
 
-        {{#if is_mutual}}
-          {{!-- create private conversation with person --}}
-          <span class="profile_button">
-            <i id="message_button" class="entypo profile-header-icon mail" title="{{t 'people.message'}}" data-placement="bottom" data-toggle="modal" data-target="#conversationModal"></i>
-          </span>
-        {{/if}}
+      {{#if is_mutual}}
+        {{!-- create private conversation with person --}}
+        <span class="profile_button">
+          <i id="message_button" class="entypo profile-header-icon mail" title="{{t 'people.message'}}" data-placement="bottom" data-toggle="modal" data-target="#conversationModal"></i>
+        </span>
+      {{/if}}
 
-        {{#unless is_blocked}}
-          {{!-- ignore the person --}}
-          <a href="#" class="profile_button" rel="nofollow">
-            <i id="block_user_button" class="entypo profile-header-icon block block_user" title="{{t 'ignore'}}" data-placement="bottom"></i>
-          </a>
-        {{/unless}}
-      </div>
-    {{/if}}
+      {{#unless is_blocked}}
+        {{!-- ignore the person --}}
+        <a href="#" class="profile_button" rel="nofollow">
+          <i id="block_user_button" class="entypo profile-header-icon block block_user" title="{{t 'ignore'}}" data-placement="bottom"></i>
+        </a>
+      {{/unless}}
+    </div>
+  {{/if}}
 
-    <ul id="profile_nav">
-      <li {{#isCurrentProfilePage guid diaspora_id}} class="active" {{/isCurrentProfilePage}}>
-        <a href="{{urlTo 'person' guid}}" id="posts_link">
-          <i class="entypo docs"></i>
-          {{t 'profile.posts'}}
+  <ul id="profile_nav">
+    <li {{#isCurrentProfilePage guid diaspora_id}} class="active" {{/isCurrentProfilePage}}>
+      <a href="{{urlTo 'person' guid}}" id="posts_link">
+        <i class="entypo docs"></i>
+        {{t 'profile.posts'}}
+      </a>
+    </li>
+    {{#if show_photos}}
+      <li {{#isCurrentPage 'personPhotos' guid}} class="active" {{/isCurrentPage}}>
+        <a href="{{urlTo 'personPhotos' guid}}" id="photos_link">
+          <i class="entypo picture"></i>
+          {{t 'profile.photos'}}
+          <div class="badge badge-default">{{photos.count}}</div>
         </a>
       </li>
-      {{#if show_photos}}
-        <li {{#isCurrentPage 'personPhotos' guid}} class="active" {{/isCurrentPage}}>
-          <a href="{{urlTo 'personPhotos' guid}}" id="photos_link">
-            <i class="entypo picture"></i>
-            {{t 'profile.photos'}}
-            <div class="badge badge-default">{{photos.count}}</div>
+    {{/if}}
+    {{#if show_contacts}}
+      <li {{#isCurrentPage 'personContacts' guid}} class="active" {{/isCurrentPage}}>
+        {{#if is_own_profile}}
+          <a href="{{urlTo 'contacts'}}" id="contacts_link">
+            <i class="entypo users"></i>
+            {{t 'profile.contacts'}}
+            <div class="badge badge-default">{{contacts.count}}</div>
           </a>
-        </li>
-      {{/if}}
-      {{#if show_contacts}}
-        <li {{#isCurrentPage 'personContacts' guid}} class="active" {{/isCurrentPage}}>
-          {{#if is_own_profile}}
-            <a href="{{urlTo 'contacts'}}" id="contacts_link">
-              <i class="entypo users"></i>
-              {{t 'profile.contacts'}}
-              <div class="badge badge-default">{{contacts.count}}</div>
-            </a>
-          {{else}}
-            <a href="{{urlTo 'personContacts' guid}}" id="contacts_link">
-              <i class="entypo users"></i>
-              {{t 'profile.contacts'}}
-              <div class="badge badge-default">{{contacts.count}}</div>
-            </a>
-          {{/if}}
-        </li>
-      {{/if}}
-    </div>
+        {{else}}
+          <a href="{{urlTo 'personContacts' guid}}" id="contacts_link">
+            <i class="entypo users"></i>
+            {{t 'profile.contacts'}}
+            <div class="badge badge-default">{{contacts.count}}</div>
+          </a>
+        {{/if}}
+      </li>
+    {{/if}}
   </div>
-{{/if}}
+</div>

app/controllers/people_controller.rb

@@ -84,7 +84,7 @@ class PeopleController < ApplicationController
         end
         gon.preloads[:person] = @person_json
         gon.preloads[:photos] = {
-          count: photos_from(@person, :all).count(:all)
+          count: Photo.visible(current_user, @person).count(:all)
         }
         gon.preloads[:contacts] = {
           count: Contact.contact_contacts_for(current_user, @person).count(:all),
@@ -146,7 +146,7 @@ class PeopleController < ApplicationController
           @contacts_of_contact = Contact.contact_contacts_for(current_user, @person)
           gon.preloads[:person] = PersonPresenter.new(@person, current_user).full_hash_with_profile
           gon.preloads[:photos] = {
-            count: photos_from(@person, :all).count(:all)
+            count: Photo.visible(current_user, @person).count(:all)
           }
           gon.preloads[:contacts] = {
             count: @contacts_of_contact.count(:all),
@@ -224,14 +224,6 @@ class PeopleController < ApplicationController
     @person.try(:remote?) && !user_signed_in?
   end
 
-  def photos_from(person, limit)
-    @photos ||= if user_signed_in?
-      current_user.photos_from(person, limit: limit)
-    else
-      Photo.where(author_id: person.id, public: true)
-    end.order('created_at desc')
-  end
-
   def mark_corresponding_notifications_read
     Notification.where(recipient_id: current_user.id, target_type: "Person", target_id: @person.id, unread: true).each do |n|
       n.set_read_state( true )

app/controllers/photos_controller.rb

@@ -3,7 +3,7 @@
 #   the COPYRIGHT file.
 
 class PhotosController < ApplicationController
-  before_action :authenticate_user!, :except => :show
+  before_action :authenticate_user!, except: %i(show index)
   respond_to :html, :json
 
   def show
@@ -19,15 +19,16 @@ class PhotosController < ApplicationController
   def index
     @post_type = :photos
     @person = Person.find_by_guid(params[:person_id])
+    authenticate_user! if @person.try(:remote?) && !user_signed_in?
 
     if @person
-      @contact = current_user.contact_for(@person)
-      @posts = current_user.photos_from(@person, max_time: max_time).order('created_at desc')
+      @contact = current_user.contact_for(@person) if user_signed_in?
+      @posts = Photo.visible(current_user, @person, :all, max_time)
       respond_to do |format|
         format.all do
           gon.preloads[:person] = PersonPresenter.new(@person, current_user).full_hash_with_profile
           gon.preloads[:photos] = {
-            count: current_user.photos_from(@person, limit: :all).count(:all)
+            count: Photo.visible(current_user, @person).count(:all)
           }
           gon.preloads[:contacts] = {
             count: Contact.contact_contacts_for(current_user, @person).count(:all),

app/models/photo.rb

@@ -145,4 +145,13 @@ class Photo < ActiveRecord::Base
   def mutable?
     true
   end
+
+  def self.visible(current_user, person, limit=:all, max_time=nil)
+    photos = if current_user
+               current_user.photos_from(person, limit: limit, max_time: max_time)
+             else
+               Photo.where(author_id: person.id, public: true)
+             end
+    photos.order("created_at desc")
+  end
 end

spec/controllers/people_controller_spec.rb

@@ -205,19 +205,6 @@ describe PeopleController, :type => :controller do
       expect(response.body).not_to include(profile.first_name)
     end
 
-    it "doesn't leak photos in the sidebar" do
-      private_photo = @user.post(:photo, user_file: uploaded_photo, to: @aspect.id, public: false)
-      public_photo = @user.post(:photo, user_file: uploaded_photo, to: @aspect.id, public: true)
-      allow(@user.person).to receive(:remote?) { false }
-
-      sign_out :user
-      get :show, id: @user.person.to_param
-
-      expect(response).to be_success
-      expect(assigns(:photos)).not_to include private_photo
-      expect(assigns(:photos)).to include public_photo
-    end
-
     it "displays the correct number of photos" do
       16.times do |i|
         eve.post(:photo, :user_file => uploaded_photo, :to => eve.aspects.first.id, :public => true)

spec/controllers/photos_controller_spec.rb

@@ -130,6 +130,49 @@ describe PhotosController, :type => :controller do
 
       expect(assigns[:posts]).to be_empty
     end
+
+    context "with no user signed in" do
+      before do
+        sign_out :user
+        @person = bob.person
+      end
+
+      it "succeeds" do
+        get :index, person_id: @person.to_param
+        expect(response.status).to eq(200)
+      end
+
+      it "succeeds on the mobile site" do
+        get :index, person_id: @person.to_param, format: :mobile
+        expect(response).to be_success
+      end
+
+      it "forces to sign in if the person is remote" do
+        p = FactoryGirl.create(:person)
+
+        get :index, person_id: p.to_param
+        expect(response).to be_redirect
+        expect(response).to redirect_to new_user_session_path
+      end
+
+      it "displays the correct number of photos" do
+        16.times do
+          eve.post(:photo, user_file: uploaded_photo, to: eve.aspects.first.id, public: true)
+        end
+        get :index, person_id: eve.person.to_param
+        expect(response.body).to include '"photos":{"count":16}'
+
+        eve.post(:photo, user_file: uploaded_photo, to: eve.aspects.first.id, public: false)
+        get :index, person_id: eve.person.to_param
+        expect(response.body).to include '"photos":{"count":16}'
+      end
+
+      it "displays a person's pictures" do
+        get :index, person_id: bob.person.guid.to_s
+        expect(assigns[:person]).to eq(bob.person)
+        expect(assigns[:posts]).to eq([@bobs_photo])
+      end
+    end
   end
 
   describe '#edit' do

spec/models/photo_spec.rb

@@ -298,4 +298,20 @@ describe Photo, :type => :model do
       @photo.receive_public
     end
   end
+
+  describe "#visible" do
+    context "with a current user" do
+      it "calls photos_from" do
+        expect(@user).to receive(:photos_from).with(@user.person, limit: :all, max_time: nil).and_call_original
+        Photo.visible(@user, @user.person)
+      end
+    end
+
+    context "without a current user" do
+      it "returns all public photos" do
+        expect(Photo).to receive(:where).with(author_id: @user.person.id, public: true).and_call_original
+        Photo.visible(nil, @user.person)
+      end
+    end
+  end
 end