pass current request to avatar url for API helper

also, use default protocol for fallback avatar url

instead of assuming HTTPS

Change-Id: Id3bac44ad252868195974202fa7d110db3ceeba1
Reviewed-on: https://gerrit.instructure.com/72604
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>

app/helpers/avatar_helper.rb

@@ -74,9 +74,14 @@ module AvatarHelper
   end
 
   def avatar_url_for_user(user, blank_fallback=false)
-    default_avatar = User.avatar_fallback_url(blank_fallback ? '/images/blank.png' : User.default_avatar_fallback)
+    default_avatar = User.avatar_fallback_url(
+        blank_fallback ? '/images/blank.png' : User.default_avatar_fallback,
+        request)
     url = if user.account.service_enabled?(:avatars)
-      user.avatar_url(nil, (@domain_root_account && @domain_root_account.settings[:avatars] || 'enabled'), default_avatar)
+      user.avatar_url(nil,
+                      (@domain_root_account && @domain_root_account.settings[:avatars] || 'enabled'),
+                      default_avatar,
+                      request)
     else
       default_avatar
     end

app/models/user.rb

@@ -1308,7 +1308,7 @@ class User < ActiveRecord::Base
   def self.avatar_fallback_url(fallback=nil, request=nil)
     return fallback if fallback == '%{fallback}'
     if fallback and uri = URI.parse(fallback) rescue nil
-      uri.scheme ||= request ? request.protocol[0..-4] : "https" # -4 to chop off the ://
+      uri.scheme ||= request ? request.protocol[0..-4] : HostUrl.protocol # -4 to chop off the ://
       if HostUrl.cdn_host
         uri.host = HostUrl.cdn_host
       elsif request && !uri.host

spec/apis/v1/discussion_topics_api_spec.rb

@@ -90,6 +90,7 @@ end
 
 describe DiscussionTopicsController, type: :request do
   include Api::V1::User
+  include AvatarHelper
 
   context 'locked api item' do
     include_examples 'a locked api item'
@@ -114,11 +115,7 @@ describe DiscussionTopicsController, type: :request do
     course_with_teacher(:active_all => true, :user => user_with_pseudonym)
   end
 
-  # needed for user_display_json
-  def avatar_url_for_user(user, *a)
-    User.avatar_fallback_url
-  end
-
+  # need for user_display_json
   def blank_fallback
     nil
   end
@@ -264,8 +261,7 @@ describe DiscussionTopicsController, type: :request do
       @sub = create_subtopic(@topic, :title => "Sub topic", :message => "<p>i'm subversive</p>")
     end
 
-    before :each do
-      @response_json =
+    let(:response_json) do
                  {"read_state"=>"read",
                   "unread_count"=>0,
                   "podcast_url"=>"/feeds/topics/#{@topic.id}/enrollment_randomness.rss",
@@ -331,7 +327,7 @@ describe DiscussionTopicsController, type: :request do
         expect(json.size).to eq 2
         # get rid of random characters in podcast url
         json.last["podcast_url"].gsub!(/_[^.]*/, '_randomness')
-        expect(json.last).to eq @response_json.merge("subscribed" => @sub.subscribed?(@user))
+        expect(json.last).to eq response_json.merge("subscribed" => @sub.subscribed?(@user))
       end
 
       it "should search discussion topics by title" do
@@ -450,7 +446,7 @@ describe DiscussionTopicsController, type: :request do
 
         # get rid of random characters in podcast url
         json["podcast_url"].gsub!(/_[^.]*/, '_randomness')
-        expect(json).to eq @response_json.merge("subscribed" => @topic.subscribed?(@user))
+        expect(json).to eq response_json.merge("subscribed" => @topic.subscribed?(@user))
       end
 
       it "should require course to be published for students" do
@@ -2180,8 +2176,8 @@ describe DiscussionTopicsController, type: :request do
         expect(json['unread_entries'].sort).to eq (@topic.discussion_entries - [@root2, @reply3] - @topic.discussion_entries.select { |e| e.user == @user }).map(&:id).sort
 
         expect(json['participants'].sort_by { |h| h['id'] }).to eq [
-          { 'id' => @student.id, 'display_name' => @student.short_name, 'avatar_image_url' => User.avatar_fallback_url, "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@student.id}" },
-          { 'id' => @teacher.id, 'display_name' => @teacher.short_name, 'avatar_image_url' => User.avatar_fallback_url, "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@teacher.id}" },
+          { 'id' => @student.id, 'display_name' => @student.short_name, 'avatar_image_url' => User.avatar_fallback_url(nil, request), "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@student.id}" },
+          { 'id' => @teacher.id, 'display_name' => @teacher.short_name, 'avatar_image_url' => User.avatar_fallback_url(nil, request), "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@teacher.id}" },
         ].sort_by { |h| h['id'] }
 
         reply_reply1_attachment_json = {

spec/apis/v1/files_controller_api_spec.rb

@@ -340,7 +340,7 @@ describe "Files API", type: :request do
         {
           "id" => @user.id,
           "display_name" => @user.short_name,
-          "avatar_image_url" => User.avatar_fallback_url,
+          "avatar_image_url" => User.avatar_fallback_url(nil, request),
           "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@user.id}"
         }
       ]
@@ -383,7 +383,7 @@ describe "Files API", type: :request do
         {
           "id" => @user.id,
           "display_name" => @user.short_name,
-          "avatar_image_url" => User.avatar_fallback_url,
+          "avatar_image_url" => User.avatar_fallback_url(nil, request),
           "html_url" => "http://www.example.com/about/#{@user.id}"
         }
       ]
@@ -729,7 +729,7 @@ describe "Files API", type: :request do
       expect(json['user']).to eql({
         "id" => @user.id,
         "display_name" => @user.short_name,
-        "avatar_image_url" => User.avatar_fallback_url,
+        "avatar_image_url" => User.avatar_fallback_url(nil, request),
         "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@user.id}"
       })
     end

spec/apis/v1/pages_api_spec.rb

@@ -20,9 +20,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../locked_spec')
 
 describe "Pages API", type: :request do
   include Api::V1::User
-  def avatar_url_for_user(user, *a)
-    User.avatar_fallback_url
-  end
+  include AvatarHelper
 
   def blank_fallback
     nil

spec/apis/v1/peer_reviews_api_spec.rb

@@ -200,7 +200,7 @@ describe PeerReviewsApiController, type: :request do
 
       @assessment_with_user = {"assessor" => {"id"=>@student2.id,
                                               "display_name"=>"User",
-                                              "avatar_image_url"=>"https://localhost/images/messages/avatar-50.png",
+                                              "avatar_image_url"=>"http://www.example.com/images/messages/avatar-50.png",
                                               "html_url"=>"http://www.example.com/courses/#{@course.id}/users/#{@student2.id}"},
                                "assessor_id"=>@student2.id,
                                "asset_id"=>@submission.id,
@@ -208,7 +208,7 @@ describe PeerReviewsApiController, type: :request do
                                "id"=>@assessment_request.id,
                                "user" => {"id"=>@student1.id,
                                           "display_name"=>"User",
-                                          "avatar_image_url"=>"https://localhost/images/messages/avatar-50.png",
+                                          "avatar_image_url"=>"http://www.example.com/images/messages/avatar-50.png",
                                           "html_url"=>"http://www.example.com/courses/#{@course.id}/users/#{@student1.id}"},
                                "user_id"=>@student1.id,
                                "workflow_state"=>@assessment_request.workflow_state}
@@ -239,7 +239,7 @@ describe PeerReviewsApiController, type: :request do
                                   "id"=>comment.id,
                                   "author"=>{"id"=>@student2.id,
                                              "display_name"=>@student2.name,
-                                             "avatar_image_url"=>"https://localhost/images/messages/avatar-50.png",
+                                             "avatar_image_url"=>"http://www.example.com/images/messages/avatar-50.png",
                                              "html_url"=>"http://www.example.com/courses/#{@course.id}/users/#{@student2.id}"}}],
        "user_id"=>@student1.id,
        "workflow_state"=>@assessment_request.workflow_state}
@@ -370,7 +370,7 @@ describe PeerReviewsApiController, type: :request do
                                "id"=>@assessment_request.id,
                                "user" => {"id"=>@student1.id,
                                           "display_name"=>"User",
-                                          "avatar_image_url"=>"https://localhost/images/messages/avatar-50.png",
+                                          "avatar_image_url"=>"http://www.example.com/images/messages/avatar-50.png",
                                           "html_url"=>"http://www.example.com/courses/#{@course.id}/users/#{@student1.id}"},
                                "user_id"=>@student1.id,
                                "workflow_state"=>@assessment_request.workflow_state})

spec/apis/v1/stream_items_api_spec.rb

@@ -363,7 +363,7 @@ describe UsersController, type: :request do
           'id' => @teacher.id,
           'display_name' => 'teacher',
           'html_url' => "http://www.example.com/courses/#{@course.id}/users/#{@teacher.id}",
-          'avatar_image_url' => User.avatar_fallback_url
+          'avatar_image_url' => User.avatar_fallback_url(nil, request)
         },
         'author_name' => 'teacher',
         'author_id' => @teacher.id,
@@ -377,7 +377,7 @@ describe UsersController, type: :request do
           'id' => @user.id,
           'display_name' => 'User',
           'html_url' => "http://www.example.com/courses/#{@course.id}/users/#{@user.id}",
-          'avatar_image_url' => User.avatar_fallback_url
+          'avatar_image_url' => User.avatar_fallback_url(nil, request)
         },
         'author_name' => 'User',
         'author_id' => @user.id,
@@ -471,7 +471,7 @@ describe UsersController, type: :request do
           'id' => @teacher.id,
           'display_name' => 'teacher',
           'html_url' => "http://www.example.com/courses/#{@course.id}/users/#{@teacher.id}",
-          'avatar_image_url' => User.avatar_fallback_url
+          'avatar_image_url' => User.avatar_fallback_url(nil, request)
         },
         'author_name' => 'teacher',
         'author_id' => @teacher.id,
@@ -485,7 +485,7 @@ describe UsersController, type: :request do
           'id' => @user.id,
           'display_name' => 'User',
           'html_url' => "http://www.example.com/courses/#{@course.id}/users/#{@user.id}",
-          'avatar_image_url' => User.avatar_fallback_url
+          'avatar_image_url' => User.avatar_fallback_url(nil, request)
         },
         'author_name' => 'User',
         'author_id' => @user.id,

spec/apis/v1/submissions_api_spec.rb

@@ -506,7 +506,7 @@ describe 'Submissions API', type: :request do
       "id" => @student.id,
       "display_name" => "User",
       "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@student.id}",
-      "avatar_image_url" => User.avatar_fallback_url
+      "avatar_image_url" => User.avatar_fallback_url(nil, request)
     })
   end
 
@@ -668,7 +668,7 @@ describe 'Submissions API', type: :request do
               "id" => @teacher.id,
               "display_name" => "User",
               "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@teacher.id}",
-              "avatar_image_url" => User.avatar_fallback_url
+              "avatar_image_url" => User.avatar_fallback_url(nil, request)
            },
            "author_name"=>"User",
            "id" => comment.id,
@@ -939,7 +939,7 @@ describe 'Submissions API', type: :request do
              "id" => @teacher.id,
              "display_name" => "User",
              "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@teacher.id}",
-             "avatar_image_url" => User.avatar_fallback_url
+             "avatar_image_url" => User.avatar_fallback_url(nil, request)
            },
            "author_name"=>"User",
            "id"=>comment.id,
@@ -1331,7 +1331,7 @@ describe 'Submissions API', type: :request do
 
         user = json.first['user']
         expect(user['display_name']).to eql @student.name
-        expect(user['avatar_image_url']).to eql "https://localhost/images/messages/avatar-50.png"
+        expect(user['avatar_image_url']).to eql "http://www.example.com/images/messages/avatar-50.png"
         expect(user['html_url']).to eql polymorphic_url([@course, @student])
       end
     end
@@ -3355,7 +3355,7 @@ describe 'Submissions API', type: :request do
         expect(json).to match_array(
           [{"id"=>@student1.id,
             "display_name"=>"User",
-            "avatar_image_url"=>"https://localhost/images/messages/avatar-50.png",
+            "avatar_image_url"=>"http://www.example.com/images/messages/avatar-50.png",
             "html_url"=>"http://www.example.com/courses/#{@course.id}/users/#{@student1.id}",
             "in_moderation_set"=>true,
             "selected_provisional_grade_id"=>pg.id,
@@ -3370,7 +3370,7 @@ describe 'Submissions API', type: :request do
                 "speedgrader_url"=>"http://www.example.com/courses/#{@course.id}/gradebook/speed_grader?assignment_id=#{@assignment.id}#%7B%22student_id%22:#{@student1.id},%22provisional_grade_id%22:#{pg.id}%7D"}]},
            {"id"=>@student2.id,
             "display_name"=>"User",
-            "avatar_image_url"=>"https://localhost/images/messages/avatar-50.png",
+            "avatar_image_url"=>"http://www.example.com/images/messages/avatar-50.png",
             "html_url"=>"http://www.example.com/courses/#{@course.id}/users/#{@student2.id}",
             "in_moderation_set"=>false,
             "selected_provisional_grade_id"=>nil,

spec/apis/v1/user_profiles_api_spec.rb

@@ -65,7 +65,7 @@ describe "User Profile API", type: :request do
       'primary_email' => nil,
       'title' => nil,
       'bio' => nil,
-      'avatar_url' => new_user.gravatar_url,
+      'avatar_url' => new_user.gravatar_url(50, nil, request),
       'time_zone' => 'Etc/UTC',
       'locale' => nil
     })
@@ -84,7 +84,7 @@ describe "User Profile API", type: :request do
       'integration_id' => nil,
       'primary_email' => 'nobody@example.com',
       'login_id' => 'nobody@example.com',
-      'avatar_url' => @admin.gravatar_url,
+      'avatar_url' => @admin.gravatar_url(50, nil, request),
       'calendar' => { 'ics' => "http://www.example.com/feeds/calendars/user_#{@admin.uuid}.ics" },
       'title' => nil,
       'bio' => nil,
@@ -107,7 +107,7 @@ describe "User Profile API", type: :request do
       'integration_id' => nil,
       'primary_email' => 'pvuser@example.com',
       'login_id' => 'pvuser@example.com',
-      'avatar_url' => @student.gravatar_url,
+      'avatar_url' => @student.gravatar_url(50, nil, request),
       'calendar' => { 'ics' => "http://www.example.com/feeds/calendars/user_#{@student.uuid}.ics" },
       'title' => nil,
       'bio' => nil,
@@ -128,7 +128,7 @@ describe "User Profile API", type: :request do
       'integration_id' => nil,
       'primary_email' => 'pvuser@example.com',
       'login_id' => 'pvuser@example.com',
-      'avatar_url' => @student.gravatar_url,
+      'avatar_url' => @student.gravatar_url(50, nil, request),
       'calendar' => { 'ics' => "http://www.example.com/feeds/calendars/user_#{@student.uuid}.ics" },
       'title' => nil,
       'bio' => nil,

spec/apis/v1/users_api_spec.rb

@@ -52,6 +52,7 @@ describe Api::V1::User do
   before :each do
     @test_api = TestUserApi.new
     @test_api.services_enabled = []
+    @test_api.request.protocol = 'http'
   end
 
   context 'user_json' do

spec/helpers/avatar_helper_spec.rb

@@ -26,7 +26,7 @@ describe AvatarHelper do
     let_once(:user) {user_model(short_name: "test user")}
     let(:services) {{avatars: true}}
     let(:avatar_size) {50}
-    let(:request) { Rack::Request.new(Rack::MockRequest.env_for("http://test.host/")) }
+    let(:request) { ActionDispatch::Request.new(Rack::MockRequest.env_for("http://test.host/")) }
 
     def service_enabled?(type)
       services[type]

spec/models/user_spec.rb

@@ -1138,6 +1138,8 @@ describe User do
     end
 
     it "should return a useful avatar_fallback_url" do
+      HostUrl.stubs(:protocol).returns('https')
+
       expect(User.avatar_fallback_url).to eq(
         "https://#{HostUrl.default_host}/images/messages/avatar-50.png"
       )