Merge pull request #645 from coopdevs/validate_image_size

Add avatar file size validation (max 5 MB)

app/assets/javascripts/application/avatar.js

@@ -38,14 +38,14 @@ $(function () {
 
     // on submit take the parameters of the box to crop the avatar
     $('#form_photo').on("submit", () => {
-      let total_width = parseInt(getComputedStyle(document.getElementById("containerCrop")).width);
-      let photo_width = parseInt(getComputedStyle(document.getElementById("foto")).width);
+      let total_width = parseInt(getComputedStyle(document.getElementById("container_crop")).width);
+      let photo_width = parseInt(getComputedStyle(document.getElementById("original_img")).width);
       let left_displacement = total_width - photo_width;
 
       $('#height_offset').val(parseInt(panel.css('top')) - 15);
       $('#width_offset').val(parseInt(panel.css('left'))  - 15 - (left_displacement/2));
       $('#height_width').val(parseInt(panel.css('width')));
-      $('#original_width').val($('#foto').width());
+      $('#original_width').val($('#original_img').width());
     });
 
     function resize(e) {
@@ -93,7 +93,7 @@ $(function () {
     }
 
     function preview_image_modal() {
-      var preview = document.querySelector('#foto');
+      var preview = document.querySelector('#original_img');
       var file = document.querySelector('#avatar-js').files[0];
       var reader = new FileReader();
 

app/assets/stylesheets/application/avatar.scss

@@ -6,8 +6,8 @@
 
 #crop_panel {
   position: absolute;
-  width: 140px;
-  height: 140px;
+  width: 200px;
+  height: 200px;
   border-left: 4px dashed black;
   cursor: move;
   touch-action: none;
@@ -21,7 +21,7 @@
   right: 0;
   width: 16px;
   height: 100%;
-  cursor: w-resize;
+  cursor: ew-resize;
 }
 
 #crop_panel::after {
@@ -32,5 +32,5 @@
   bottom: 0;
   width: 100%;
   height: 16px;
-  cursor: n-resize;
+  cursor: ns-resize;
 }

app/controllers/users_controller.rb

@@ -66,14 +66,12 @@ def update
   end
 
   def update_avatar
-    avatar = params[:avatar]
+    operation = AvatarGenerator.new(current_user, params)
 
-    if avatar && content_type_permitted(avatar.content_type)
-      current_user.avatar.purge if current_user.avatar.attached?
-
-      crop_image_and_save(current_user, avatar)
+    if operation.errors.empty?
+      operation.call
     else
-      flash[:error] = t 'users.show.invalid_format'
+      flash[:error] = operation.errors.join("<br>")
     end
 
     redirect_to current_user
@@ -130,28 +128,4 @@ def redirect_to_after_create
                                  name: @user.username)
     end
   end
-
-  def crop_image_and_save(user, avatar)
-    orig_width = params[:original_width].to_i
-    width      = params[:height_width].to_i
-    left       = params[:width_offset].to_i
-    top        = params[:height_offset].to_i
-
-    image_processed = ImageProcessing::MiniMagick.
-                      source(avatar.tempfile).
-                      resize_to_fit(orig_width, nil).
-                      crop("#{width}x#{width}+#{left}+#{top}!").
-                      convert("png").
-                      call
-
-    user.avatar.attach(
-      io: image_processed,
-      filename: user.username,
-      content_type: avatar.content_type
-    )
-  end
-
-  def content_type_permitted(avatar_content_type)
-    %w[image/jpeg image/pjpeg image/png image/x-png].include? avatar_content_type
-  end
 end

app/services/avatar_generator.rb

@@ -0,0 +1,51 @@
+class AvatarGenerator
+  MAX_SIZE = 5
+  CONTENT_TYPES = %w[image/jpeg image/pjpeg image/png image/x-png]
+
+  attr_accessor :errors
+
+  def initialize(user, params)
+    @user   = user
+    @params = params
+    @avatar = params[:avatar]
+    @errors = []
+
+    validate_avatar
+  end
+
+  def call
+    @user.avatar.purge if @user.avatar.attached?
+
+    @user.avatar.attach(
+      io: process_image,
+      filename: @user.username,
+      content_type: @avatar.content_type
+    )
+  end
+
+  private
+
+  def process_image
+    orig_width = @params[:original_width].to_i
+    width      = @params[:height_width].to_i
+    left       = @params[:width_offset].to_i
+    top        = @params[:height_offset].to_i
+
+    ImageProcessing::MiniMagick.
+      source(@avatar.tempfile).
+      resize_to_fit(orig_width, nil).
+      crop("#{width}x#{width}+#{left}+#{top}!").
+      convert("png").
+      call
+  end
+
+  def validate_avatar
+    if CONTENT_TYPES.exclude?(@avatar.content_type)
+      @errors << I18n.t("users.show.invalid_format")
+    end
+
+    if @avatar.size.to_f > MAX_SIZE.megabytes
+      @errors << I18n.t("users.avatar.max_size_warning", size: MAX_SIZE)
+    end
+  end
+end

app/views/layouts/_messages.html.erb

@@ -5,7 +5,7 @@
     <button type="button" class="close" data-dismiss="alert">x</button>
     <ul>
       <li>
-        <%= value %>
+        <%= value.html_safe %>
       </li>
     </ul>
   </div>

app/views/users/_avatar.html.erb

@@ -16,25 +16,24 @@
         <div class="modal-dialog">
           <div class="modal-content">
             <div class="modal-header">
-              <h4 class="modal-title">
-                <%= t ".crop_the_image" %>
-              </h4>
+              <h4 class="modal-title"><%= t ".crop_the_image" %></h4>
             </div>
             <div class="modal-body">
-              <div id="containerCrop" class="col-12">
+              <div id="container_crop" class="col-12">
                 <div id="crop_panel"></div>
-                <img id="foto" alt="" />
+                <img id="original_img">
               </div>
             </div>
             <div class="modal-footer">
               <div class="form-actions">
-                <button type="button" class="btn btn-default" data-dismiss="modal">
-                  <%= t 'users.new.cancel' %>
-                </button>
-                <%= f.hidden_field :height_width, value: 140 %>
+                <%= f.hidden_field :height_width, value: 200 %>
                 <%= f.hidden_field :width_offset, value: 1 %>
                 <%= f.hidden_field :height_offset, value: 1 %>
                 <%= f.hidden_field :original_width, value: 1 %>
+
+                <%= f.button :submit, class: "btn btn-default", data: { dismiss: "modal" } do %>
+                  <%= t("users.new.cancel") %>
+                <% end %>
                 <%= f.button :submit, class: "btn btn-primary" do %>
                   <%= t("global.save") %>
                 <% end %>

config/locales/ca.yml

@@ -505,6 +505,7 @@ ca:
     avatar:
       change_your_image: Canvia la teva imatge
       crop_the_image: Retalla la imatge
+      max_size_warning: La imatge és massa gran, el màxim permès és de %{size}MB
     edit:
       edit_user: Canviar usuari
     form:

config/locales/en.yml

@@ -502,6 +502,10 @@ en:
     new:
       error_amount: Time must be greater than 0
   users:
+    avatar:
+      change_your_image: Change your image
+      crop_the_image: Crop the image
+      max_size_warning: Image is too big, max allowed is %{size}MB
     edit:
       edit_user: Update user
     form:
@@ -526,9 +530,6 @@ en:
       create_more_users_button: Create and add another user
       new_user: New user
       user_created_add: User %{uid} %{name} saved, now create next one
-    avatar:
-      change_your_image: Change your image
-      crop_the_image: Crop the image
     show:
       account: Last transactions
       accounts: Accounts

config/locales/es.yml

@@ -505,6 +505,7 @@ es:
     avatar:
       change_your_image: Cambia tu imagen
       crop_the_image: Recortar la imagen
+      max_size_warning: La imagen es demasiado grande, el máximo permitido es %{size}MB
     edit:
       edit_user: Cambiar usuario
     form:

config/locales/eu.yml

@@ -505,6 +505,7 @@ eu:
     avatar:
       change_your_image: 
       crop_the_image: 
+      max_size_warning: 
     edit:
       edit_user: Erabiltzailea aldatu
     form:

config/locales/gl.yml

@@ -505,6 +505,7 @@ gl:
     avatar:
       change_your_image: 
       crop_the_image: 
+      max_size_warning: 
     edit:
       edit_user: Actualizar persoa usuaria
     form:

config/locales/pt-BR.yml

@@ -505,6 +505,7 @@ pt-BR:
     avatar:
       change_your_image: 
       crop_the_image: 
+      max_size_warning: 
     edit:
       edit_user: Trocar usuário
     form:

spec/controllers/users_controller_spec.rb

@@ -409,18 +409,31 @@
     context "with a normal logged user" do
       before { login(user) }
 
-      it "change the photo attached @user" do
-        path = Rails.root.join("docs/images/ofertas.png")
-        uploaded_file = Rack::Test::UploadedFile.new(path, "image/png")
+      let(:uploaded_file) do
+        Rack::Test::UploadedFile.new(
+          Rails.root.join("docs/images/ofertas.png"),
+          "image/png"
+        )
+      end
 
+      it "change the photo attached @user" do
         put :update_avatar, params: { avatar: uploaded_file, original_width: 500, height_width: 140 }
 
         expect(user.avatar.attached?).to eq true
       end
 
-      it "don't change the photo attached if the format isn't valid" do
-        path = Rails.root.join("spec/controllers/users_controller_spec.rb")
-        uploaded_file = Rack::Test::UploadedFile.new(path)
+      it "don't change the photo attached if the mime_type isn't valid" do
+        uploaded_file = Rack::Test::UploadedFile.new(
+          Rails.root.join("spec/controllers/users_controller_spec.rb")
+        )
+
+        put :update_avatar, params: { avatar: uploaded_file }
+
+        expect(flash[:error]).not_to be_empty
+      end
+
+      it "don't change the photo attached if the file size it too big" do
+        allow_any_instance_of(ActionDispatch::Http::UploadedFile).to receive(:size) { AvatarGenerator::MAX_SIZE.megabytes + 1.megabyte }
 
         put :update_avatar, params: { avatar: uploaded_file }