Merge pull request #1513 from Retrospring/fix/image-upload-constraints

app/controllers/settings/profile_picture_controller.rb

@@ -12,9 +12,10 @@ class Settings::ProfilePictureController < ApplicationController
       text += t(".notice.profile_header") if user_attributes[:profile_header]
       flash[:success] = text
     else
-      flash[:error] = t(".error")
+      # CarrierWave resets the image to the default upon an error
+      current_user.reload
     end
 
-    redirect_to settings_profile_path
+    render "settings/profile/edit"
   end
 end

app/uploaders/base_uploader.rb

@@ -8,12 +8,14 @@ class BaseUploader < CarrierWave::Uploader::Base
   # Store original size
   version :original
 
-  # Process cropping on upload
+  process :remove_animation
   process :cropping
 
-  def store_dir
-    "/uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
-  end
+  def content_type_whitelist = %w[image/jpeg image/gif image/png]
+
+  def store_dir = "/uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
+
+  def size_range = (1.byte)..(5.megabytes)
 
   def paperclip_path
     return "/users/:attachment/:id_partition/:style/:basename.:extension" if APP_CONFIG["fog"].blank?
@@ -31,4 +33,10 @@ class BaseUploader < CarrierWave::Uploader::Base
       image.crop "#{w}x#{h}+#{x}+#{y}"
     end
   end
+
+  def remove_animation
+    return unless content_type == "image/gif"
+
+    manipulate!(&:collapse!)
+  end
 end

app/uploaders/profile_header_uploader.rb

@@ -1,7 +1,7 @@
 class ProfileHeaderUploader < BaseUploader
-  def default_url(*args)
-    "/images/header/#{[version_name || args.first, 'no_header.jpg'].compact.join('/')}"
-  end
+  def default_url(*args) = "/images/header/#{[version_name || args.first, 'no_header.jpg'].compact.join('/')}"
+
+  def size_range = (1.byte)..(10.megabytes)
 
   version :web do
     process resize_to_fit: [1500, 350]

app/views/settings/profile/edit.html.haml

@@ -7,7 +7,7 @@
           .flex-shrink-0
             = render AvatarComponent.new(user: current_user, size: "lg", classes: ["me-3"])
           .flex-grow-1
-            = f.file_field :profile_picture, accept: APP_CONFIG[:accepted_image_formats].join(","), data: { cropper_target: "input", action: "cropper#change" }
+            = f.file_field :profile_picture, accept: current_user.profile_picture.content_type_whitelist.join(','), data: { cropper_target: "input", action: "cropper#change" }
 
         .row.d-none{ data: { cropper_target: "controls" } }
           .col-sm-10.col-md-8
@@ -22,7 +22,7 @@
           .col-xs-12.col-md-6
             %img.mw-100.me-3{ src: current_user.profile_header.url(:mobile) }
           .col-xs-12.col-md-6.mt-3.mt-sm-0.ps-3.pe-3
-            = f.file_field :profile_header, accept: APP_CONFIG[:accepted_image_formats].join(","), data: { cropper_target: "input", action: "cropper#change" }
+            = f.file_field :profile_header, accept: current_user.profile_header.content_type_whitelist.join(','), data: { cropper_target: "input", action: "cropper#change" }
 
         .row.d-none{ data: { cropper_target: "controls" } }
           .col-sm-10.col-md-8

config/justask.yml.example

@@ -81,19 +81,6 @@ hcaptcha:
 # TOTP Drift period in seconds
 otp_drift_period: 30
 
-# This list controls the "accept" attribute on file upload fields
-# This ensures mobile users get an appropriate file picker (one for only images)
-# as well as preventing the upload of videos or formats we don't support
-# including making iOS automatically convert HEIC files to JPEG
-accepted_image_formats:
-  - image/jpeg
-  - .jpg
-  - .jpeg
-  - image/png
-  - .png
-  - image/gif
-  - .gif
-
 # This list controls which hosts are excempt from the linkfilter
 # Note: `hostname` is always included by default
 allowed_hosts_in_markdown:

spec/controllers/settings/profile_picture_controller_spec.rb

@@ -32,7 +32,8 @@ describe Settings::ProfilePictureController, type: :controller do
 
       it "redirects to the edit_user_profile page" do
         subject
-        expect(response).to redirect_to(:settings_profile)
+        expect(response).to have_http_status(:ok)
+        expect(response).to have_rendered(:edit)
       end
     end
   end