Merge pull request #1377 from ZogStriP/avatar-work

Improved specs for avatar + added a warning whenever the uploaded image is not a square

app/assets/javascripts/discourse/templates/modal/avatar_selector.js.handlebars

@@ -21,6 +21,9 @@
       {{#if view.uploading}}
         <span>{{i18n upload_selector.uploading}} {{view.uploadProgress}}%</span>
       {{/if}}
+      {{#if view.imageIsNotASquare}}
+        <div class="warning">{{i18n user.change_avatar.image_is_not_a_square}}</div>
+      {{/if}}
     </div>
   </div>
 </div>

app/assets/javascripts/discourse/views/modal/avatar_selector_view.js

@@ -15,6 +15,7 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
   useGravatar: Em.computed.not("controller.use_uploaded_avatar"),
   canSaveAvatarSelection: Em.computed.or("useGravatar", "controller.has_uploaded_avatar"),
   saveDisabled: Em.computed.not("canSaveAvatarSelection"),
+  imageIsNotASquare : false,
 
   didInsertElement: function() {
     var view = this;
@@ -40,7 +41,10 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
 
     // when a file has been selected
     $upload.on("fileuploadadd", function (e, data) {
-      view.set("uploading", true);
+      view.setProperties({
+        uploading: true,
+        imageIsNotASquare: false
+      });
     });
 
     // when there is a progression for the upload
@@ -56,6 +60,8 @@ Discourse.AvatarSelectorView = Discourse.ModalBodyView.extend({
         has_uploaded_avatar: true,
         use_uploaded_avatar: true
       });
+      // display a warning whenever the image is not a square
+      view.set("imageIsNotASquare", data.result.width !== data.result.height);
       // in order to be as much responsive as possible, we're cheating a bit here
       // indeed, the server gives us back the url to the file we've just uploaded
       // often, this file is not a square, so we need to crop it properly

app/assets/stylesheets/application/modal.css.scss

@@ -178,8 +178,9 @@
     .archetype-option {
       margin-bottom: 20px;
     }
-
-
+    .warning {
+      color: lighten($red, 10%) !important;
+    }
   }
   .password-confirmation {
     display: none;

app/controllers/users_controller.rb

@@ -7,7 +7,7 @@ class UsersController < ApplicationController
   skip_before_filter :authorize_mini_profiler, only: [:avatar]
   skip_before_filter :check_xhr, only: [:show, :password_reset, :update, :activate_account, :authorize_email, :user_preferences_redirect, :avatar]
 
-  before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect]
+  before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect, :upload_avatar, :toggle_avatar]
 
   # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the
   #  page is going to be empty, this means that server will see an invalid CSRF and blow the session
@@ -342,13 +342,18 @@ class UsersController < ApplicationController
 
     upload = Upload.create_for(user.id, file, filesize)
 
+    user.uploaded_avatar_template = nil
     user.uploaded_avatar = upload
     user.use_uploaded_avatar = true
     user.save!
 
     Jobs.enqueue(:generate_avatars, upload_id: upload.id)
 
-    render json: { url: upload.url }
+    render json: {
+      url: upload.url,
+      width: upload.width,
+      height: upload.height,
+    }
 
   rescue FastImage::ImageFetchFailure
     render status: 422, text: I18n.t("upload.images.fetch_failure")

config/locales/client.en.yml

@@ -231,6 +231,7 @@ en:
         uploaded_avatar: "Custom picture"
         uploaded_avatar_empty: "Add a custom picture"
         upload_title: "Upload your picture"
+        image_is_not_a_square: "Warning: we've cropped your image as it's not a square."
 
       email:
         title: "Email"

config/locales/client.fr.yml

@@ -226,6 +226,7 @@ fr:
         uploading: "Image en cours d'envois..."
         gravatar: "Gravatar"
         uploaded_avatar: "Image envoyée"
+        image_is_not_a_square: "Attention : nous avons coupé l'image pour en faire un carré."
 
       email:
         title: "Email"

config/routes.rb

@@ -138,11 +138,9 @@ Discourse::Application.routes.draw do
   get 'users/:username/preferences/about-me' => 'users#preferences', constraints: {username: USERNAME_ROUTE_FORMAT}
   get 'users/:username/preferences/username' => 'users#preferences', constraints: {username: USERNAME_ROUTE_FORMAT}
   put 'users/:username/preferences/username' => 'users#username', constraints: {username: USERNAME_ROUTE_FORMAT}
-  # LEGACY ROUTE
-  get 'users/:username/avatar(/:size)' => 'users#avatar', constraints: {username: USERNAME_ROUTE_FORMAT}
-  get 'users/:username/preferences/avatar' => 'users#preferences', constraints: {username: USERNAME_ROUTE_FORMAT}
-  put 'users/:username/preferences/avatar/toggle' => 'users#toggle_avatar', constraints: {username: USERNAME_ROUTE_FORMAT}
+  get 'users/:username/avatar(/:size)' => 'users#avatar', constraints: {username: USERNAME_ROUTE_FORMAT} # LEGACY ROUTE
   post 'users/:username/preferences/avatar' => 'users#upload_avatar', constraints: {username: USERNAME_ROUTE_FORMAT}
+  put 'users/:username/preferences/avatar/toggle' => 'users#toggle_avatar', constraints: {username: USERNAME_ROUTE_FORMAT}
   get 'users/:username/invited' => 'users#invited', constraints: {username: USERNAME_ROUTE_FORMAT}
   post 'users/:username/send_activation_email' => 'users#send_activation_email', constraints: {username: USERNAME_ROUTE_FORMAT}
   get 'users/:username/activity' => 'users#show', constraints: {username: USERNAME_ROUTE_FORMAT}

spec/controllers/users_controller_spec.rb

@@ -938,4 +938,90 @@ describe UsersController do
     end
   end
 
+  describe '.upload_avatar' do
+
+    it 'raises an error when not logged in' do
+      lambda { xhr :put, :upload_avatar, username: 'asdf' }.should raise_error(Discourse::NotLoggedIn)
+    end
+
+    context 'while logged in' do
+
+      let!(:user) { log_in }
+
+      let(:avatar) do
+        ActionDispatch::Http::UploadedFile.new({
+          filename: 'logo.png',
+          tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png")
+        })
+      end
+
+      it 'raises an error when you don\'t have permission to upload an avatar' do
+        Guardian.any_instance.expects(:can_edit?).with(user).returns(false)
+        xhr :post, :upload_avatar, username: user.username
+        response.should be_forbidden
+      end
+
+      it 'rejects large images' do
+        SiteSetting.stubs(:max_image_size_kb).returns(1)
+        xhr :post, :upload_avatar, username: user.username, file: avatar
+        response.status.should eq 413
+      end
+
+      it 'is successful' do
+        upload = Fabricate(:upload)
+        Upload.expects(:create_for).returns(upload)
+        # enqueues the avatar generator job
+        Jobs.expects(:enqueue).with(:generate_avatars, { upload_id: upload.id })
+        xhr :post, :upload_avatar, username: user.username, file: avatar
+        user.reload
+        # erase the previous template
+        user.uploaded_avatar_template.should == nil
+        # link to the right upload
+        user.uploaded_avatar.id.should == upload.id
+        # automatically set "use_uploaded_avatar"
+        user.use_uploaded_avatar.should == true
+      end
+
+      it 'returns the url, width and height of the uploaded image' do
+        xhr :post, :upload_avatar, username: user.username, file: avatar
+        json = JSON.parse(response.body)
+        json['url'].should_not be_nil
+        json['width'].should == 244
+        json['height'].should == 66
+      end
+
+    end
+
+  end
+
+  describe '.toggle_avatar' do
+
+    it 'raises an error when not logged in' do
+      lambda { xhr :put, :toggle_avatar, username: 'asdf' }.should raise_error(Discourse::NotLoggedIn)
+    end
+
+    context 'while logged in' do
+
+      let!(:user) { log_in }
+
+      it 'raises an error without a use_uploaded_avatar param' do
+        lambda { xhr :put, :toggle_avatar, username: user.username }.should raise_error(ActionController::ParameterMissing)
+      end
+
+      it 'raises an error when you don\'t have permission to toggle the avatar' do
+        Guardian.any_instance.expects(:can_edit?).with(user).returns(false)
+        xhr :put, :toggle_avatar, username: user.username, use_uploaded_avatar: "true"
+        response.should be_forbidden
+      end
+
+      it 'it successful' do
+        xhr :put, :toggle_avatar, username: user.username, use_uploaded_avatar: "false"
+        user.reload.use_uploaded_avatar.should == false
+        response.should be_success
+      end
+
+    end
+
+  end
+
 end