Search code examples
ruby-on-railsrubyrails-activestorageruby-on-rails-6

Active Storage still detects attachement even after validation failure


This is an issue that has been going on for quite some time and to this day, I still have not found the solution. I have seen some similar issues but not quite what I have been experiencing.

I have been experiencing Rails Active Storage since it came out in Rails 5 but never got to actually use it in production because of this particular issue. The main issue, which was resolved when Rails 6 came out was that if you implemented any validation for a file attachment, the record would not save, but the attachment (blob) would still be saved and if your validation was on the content type (making sure it's a JPEG image for instance) then you ended up with an invalid attachment (if you uploaded a text file for instance). This would cause issue if you were to try to "display" this attachment with an image_tag for instance. Rails 6 resolves this issue by saving the attachment only when the record is actually saved into the database. This only fixes half of the issue for me.

Here is what I am still experiencing and have not found a solution yet.

Suppose you have a very basic setup. A Person model with a name, an email and an attached avatar

Class Person < ApplicationRecord
  has_one_attached :avatar
  #Check that image type is jpg or png
  validate :check_image_type

  #Remove avatar flag needed for form
  attr_accessor :remove_avatar

  #purge picture if remove picture flag was ticked on form
  after_save :purge_avatar, if: :purge_requested?

  #Returns a thumbnail version of the property picture
  def thumbnail
    return self.avatar.variant(resize:'100x100').processed 
  end

  private 
    #Validates the image type being uploaded
    def check_image_type
      if avatar.attached? && !avatar.content_type.in?(%("image/jpeg image/png"))
        errors.add(:avatar, "Invalid Avatar Format")
      end
    end

    #Was a purge of the picture requested 
    def purge_requested?
      remove_avatar == "1"
    end

    def purge_avatar
      avatar.purge_later
    end
end 

As you can see in the above code, the Person has one attached avatar. Upon saving we validate the image type making sure it's jpep or png and if not we simply add error to the record which will prevent the record from being saved.

Here is the Controller code (I omitted the index, edit, update and destroy actions)

class PeopleController < ApplicationController
  before_action :set_person, only: [:show, :edit, :update, :destroy]

  def new
    @person = Person.new
  end

  def create
    @person = Person.new(person_params)

    respond_to do |format|
      if @person.save
        format.html { redirect_to @person, notice: 'Person was successfully created.' }
        format.json { render :show, status: :created, location: @person }
      else
        format.html { render :new }
        format.json { render json: @person.errors, status: :unprocessable_entity }
      end
    end
  end

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_person
      @person = Person.find(params[:id])
    end

    # Never trust parameters from the scary internet, only allow the white list through.
    def person_params
      params.require(:person).permit(:name, :email, :avatar, :remove_avatar)
    end
end

Then in the form, I show the avatar if it exists on top of the form, and then let the user create/edit the information and select another avatar if they want.

<%= form_with(model: person, local: true) do |form| %>
  <% if person.errors.any? %>
    <div id="error_explanation">
      <h2>
        <%= pluralize(person.errors.count, "error") %> prohibited this person from being saved:
      </h2>

      <ul>
        <% person.errors.full_messages.each do |message| %>
          <li><%= message %></li>
        <% end %>
      </ul>
    </div>
  <% end %>
  <!-- Display avatar if one attach -->
  <%if person.avatar.attached?%>
    <%=image_tag(person.avatar)%>
    Remove <%=form.check_box :remove_avatar%>
  <%end%>

  <div class="field">
    <%= form.label :name %>
    <%= form.text_field :name %>
  </div>

  <div class="field">
    <%= form.label :email %>
    <%= form.text_field :email %>
  </div>

  <!-- Select Picture -->
  <div class = "field">
    <%=form.label :avatar %>
    <%= form.file_field :avatar, accept: 'image/*'%>
  </div>

  <div class="actions">
    <%= form.submit %>
  </div>
<% end %>

The expected behavior is

  • When creating a new person, no avatar is displayed (this works)
  • By selecting a proper file type (jpg or png) you can save the Person (this works)
  • By selecting a wrong file type you cannot save the person and can select another file (not working)

What happens is when you select the wrong file type, it does prevent the record from being saved and that's fine, but it still "sees" an attachment on the non-persisted record. So although there was no avatar showing when the form was initially showed on the create action, an "empty" avatar shows up when the form is re-rendered after the validation fails. This is due to the attached? method returning true even though there is no attachment on the record (since it was rejected).

Having a blank avatar simply looks a bit funky, it looks like a picture that has a broken link. However if you were to do any manipulation on the avatar itself such the thumbnail method in the Person class, this generates the following error: ActiveStorage::InvariableError

This is due to the attached? attribute being true and the avatar attribute being valid but it does not have an associated blob (picture). So trying to resize it to a thumbnail fails with an error.

I'm trying ton find a way to "clear" or reset the avatar and/or attached? attributes when the validation prevents the record from saving. Internally, Rails does what it is supposed to do (not saving the file and preserving the current one if there is one).

But what is shown on the form (if you display the avatar) is bound to confuse the user, especially if the user had an avatar before you select a new one. If you select an invalid avatar and the form rejects it, on the re-render your initial avatar isn't showing and is replaced with that picture broken link icon. This could confuse the user in thinking their previous avatar has been wiped out while this is not the case. At this point i'm not sure how to fix this without going deep into the active storage guts (which I'm not interested in doing right now).

I have tried to call purge myself or to assign null to the avatar attribute but that didn't work

def check_image_type
  if avatar.attached? && !avatar.content_type.in?(%("image/jpeg image/png"))
    errors.add(:avatar, "Invalid Avatar Format")
    avatar.purge    <---- Not working
    avatar = nil <--- Not working
    avatar = '' <--- Not working
  end
end

EDIT: I'm not necessarily trying to display a preview of what I just uploaded. As a matter of fact, I don't want to do that but Rails seems to be doing that on its own.

In my example, when you create the user, there is no avatar so none is showing, but when you try to upload an avatar of the wrong file type and the form reloads to show the error, it tries to display the failed loaded avatar. If the file uploaded is of the correct type, it saves the user info and redirects to the user list or to another screen where we could display the loaded avatar.

In the case where the user already has an avatar and you want to change it. You first open the form (with edit action) and it shows the current avatar. If I try to change it and upload an invalid file, again, the form reloads with the error but again, replaces the current valid avatar with an empty one (even though i the database, the old one still remains). In the same way, if I do upload a valid file, then the form will submit, the avatar will change and we will see it in the next screen.

To sum it up, what (I feel) the proper behavior should be is If I try to upload a file that is rejected based on validation (file type, size etc) then Rails should act as if I had not even tried to upload a file. It should scrap any remains of a "tentative attachment".

In the case of a new resource, it would still show up no avatar, but in the case of an already existing resource, it would then still show the current avatar.


Solution

  • To error prevent you can use persisted?

    <% if person.avatar.attached? && person.avatar.persisted? %>
      <%= image_tag(person.avatar)%>
      Remove <%= form.check_box :remove_avatar%>
    <%end%>
    

    And you can use this gem for ActiveStorage validation.

    Like this:

    validates :avatar, content_type: %w[image/png image/jpg image/jpeg]