Stronger direct upload model specs

This commit is contained in:
Senén Rodero Rodríguez
2017-09-20 18:22:52 +02:00
parent 346c329919
commit 661bdda41f
6 changed files with 79 additions and 52 deletions

View File

@@ -8,13 +8,13 @@ class DirectUploadsController < ApplicationController
helper_method :render_destroy_upload_link helper_method :render_destroy_upload_link
# It should return cached attachment path or attachment errors
def create def create
@direct_upload = DirectUpload.new(direct_upload_params.merge(user: current_user, attachment: params[:attachment])) @direct_upload = DirectUpload.new(direct_upload_params.merge(user: current_user, attachment: params[:attachment]))
if @direct_upload.valid? if @direct_upload.valid?
@direct_upload.save_attachment @direct_upload.save_attachment
@direct_upload.relation.set_cached_attachment_from_attachment(URI(request.url)) @direct_upload.relation.set_cached_attachment_from_attachment(URI(request.url))
render json: { cached_attachment: @direct_upload.relation.cached_attachment, render json: { cached_attachment: @direct_upload.relation.cached_attachment,
filename: @direct_upload.relation.attachment.original_filename, filename: @direct_upload.relation.attachment.original_filename,
destroy_link: render_destroy_upload_link(@direct_upload).html_safe, destroy_link: render_destroy_upload_link(@direct_upload).html_safe,
@@ -29,7 +29,6 @@ class DirectUploadsController < ApplicationController
def destroy def destroy
@direct_upload = DirectUpload.new(direct_upload_params.merge(user: current_user) ) @direct_upload = DirectUpload.new(direct_upload_params.merge(user: current_user) )
@direct_upload.relation.set_attachment_from_cached_attachment @direct_upload.relation.set_attachment_from_cached_attachment
if @direct_upload.destroy_attachment if @direct_upload.destroy_attachment

View File

@@ -7,9 +7,27 @@ class DirectUpload
:relation, :resource_relation, :relation, :resource_relation,
:attachment, :cached_attachment, :user :attachment, :cached_attachment, :user
validates_presence_of :attachment, :resource_type, :resource_relation validates_presence_of :attachment, :resource_type, :resource_relation, :user
validate :parent_resource_attachment_validations, validate :parent_resource_attachment_validations,
if: -> { attachment.present? && resource_type.present? && resource_relation.present? } if: -> { attachment.present? && resource_type.present? && resource_relation.present? && user.present? }
def initialize(attributes = {})
attributes.each do |name, value|
send("#{name}=", value)
end
if @resource_type.present? && @resource_relation.present? && (@attachment.present? || @cached_attachment.present?)
@resource = @resource_type.constantize.find_or_initialize_by(id: @resource_id)
if @resource.class.reflections[@resource_relation].macro == :has_one
@relation = @resource.send("build_#{resource_relation}", relation_attributtes)
else
@relation = @resource.send(@resource_relation).build(relation_attributtes)
end
@relation.user = user
end
end
def save_attachment def save_attachment
@relation.attachment.save @relation.attachment.save
@@ -23,24 +41,6 @@ class DirectUpload
false false
end end
def initialize(attributes = {})
attributes.each do |name, value|
send("#{name}=", value)
end
if @resource_type.present?
@resource = @resource_type.constantize.find_or_initialize_by(id: @resource_id)
end
if @resource.class.reflections[@resource_relation].macro == :has_one
@relation = @resource.send("build_#{resource_relation}", attachment: @attachment, cached_attachment: @cached_attachment)
else
@relation = @resource.send(resource_relation).build(attachment: @attachment, cached_attachment: @cached_attachment)
end
@relation.user = user
end
private private
def parent_resource_attachment_validations def parent_resource_attachment_validations
@@ -51,4 +51,12 @@ class DirectUpload
end end
end end
def relation_attributtes
{
attachment: @attachment,
cached_attachment: @cached_attachment,
user: @user
}
end
end end

View File

@@ -20,8 +20,6 @@ class Document < ActiveRecord::Base
validates :documentable_id, presence: true, if: -> { persisted? } validates :documentable_id, presence: true, if: -> { persisted? }
validates :documentable_type, presence: true, if: -> { persisted? } validates :documentable_type, presence: true, if: -> { persisted? }
after_save :remove_cached_document, if: -> { valid? && persisted? && cached_attachment.present? }
def set_cached_attachment_from_attachment(prefix) def set_cached_attachment_from_attachment(prefix)
self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem
attachment.path attachment.path
@@ -42,7 +40,7 @@ class Document < ActiveRecord::Base
attachment.instance.prefix(attachment, style) attachment.instance.prefix(attachment, style)
end end
def prefix(attachment, _style) def prefix(attachment, style)
if !attachment.instance.persisted? if !attachment.instance.persisted?
"cached_attachments/user/#{attachment.instance.user_id}" "cached_attachments/user/#{attachment.instance.user_id}"
else else
@@ -80,8 +78,4 @@ class Document < ActiveRecord::Base
end end
end end
def remove_cached_document
File.delete(cached_attachment) if File.exist?(cached_attachment)
end
end end

View File

@@ -29,9 +29,6 @@ class Image < ActiveRecord::Base
validate :validate_image_dimensions, if: -> { attachment.present? && attachment.dirty? } validate :validate_image_dimensions, if: -> { attachment.present? && attachment.dirty? }
after_create :redimension_using_origin_styles
after_save :remove_cached_image, if: -> { valid? && persisted? && cached_attachment.present? }
def set_cached_attachment_from_attachment(prefix) def set_cached_attachment_from_attachment(prefix)
self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem self.cached_attachment = if Paperclip::Attachment.default_options[:storage] == :filesystem
attachment.path attachment.path
@@ -60,13 +57,8 @@ class Image < ActiveRecord::Base
end end
end end
private private
def redimension_using_origin_styles
attachment.reprocess!
end
def imageable_class def imageable_class
imageable_type.constantize if imageable_type.present? imageable_type.constantize if imageable_type.present?
end end
@@ -102,10 +94,6 @@ class Image < ActiveRecord::Base
end end
end end
def remove_cached_image
File.delete(cached_attachment) if File.exists?(cached_attachment)
end
def attachment_of_valid_content_type? def attachment_of_valid_content_type?
attachment.present? && imageable_accepted_content_types.include?(attachment_content_type) attachment.present? && imageable_accepted_content_types.include?(attachment_content_type)
end end

View File

@@ -820,6 +820,7 @@ LOREM_IPSUM
end end
factory :direct_upload do factory :direct_upload do
user
trait :proposal do trait :proposal do
resource_type "Proposal" resource_type "Proposal"
@@ -836,6 +837,7 @@ LOREM_IPSUM
resource_relation "image" resource_relation "image"
attachment { File.new("spec/fixtures/files/clippy.jpg") } attachment { File.new("spec/fixtures/files/clippy.jpg") }
end end
initialize_with { new(attributes) }
end end
end end

View File

@@ -2,6 +2,8 @@ require 'rails_helper'
describe DirectUpload do describe DirectUpload do
context "configurations" do
it "should be valid for different kind of combinations when attachment is valid" do it "should be valid for different kind of combinations when attachment is valid" do
expect(build(:direct_upload, :proposal, :documents)).to be_valid expect(build(:direct_upload, :proposal, :documents)).to be_valid
expect(build(:direct_upload, :proposal, :image)).to be_valid expect(build(:direct_upload, :proposal, :image)).to be_valid
@@ -9,11 +11,45 @@ describe DirectUpload do
expect(build(:direct_upload, :budget_investment, :image)).to be_valid expect(build(:direct_upload, :budget_investment, :image)).to be_valid
end end
it "should not be valid for different kind of combinations when invalid atttachment content types" do it "should not be valid for different kind of combinations with invalid atttachment content types" do
expect(build(:direct_upload, :proposal, :documents, attachment: File.new("spec/fixtures/files/clippy.png"))).not_to be_valid expect(build(:direct_upload, :proposal, :documents, attachment: File.new("spec/fixtures/files/clippy.png"))).not_to be_valid
expect(build(:direct_upload, :proposal, :image, attachment: File.new("spec/fixtures/files/empty.pdf"))).not_to be_valid expect(build(:direct_upload, :proposal, :image, attachment: File.new("spec/fixtures/files/empty.pdf"))).not_to be_valid
expect(build(:direct_upload, :budget_investment, :documents, attachment: File.new("spec/fixtures/files/clippy.png"))).not_to be_valid expect(build(:direct_upload, :budget_investment, :documents, attachment: File.new("spec/fixtures/files/clippy.png"))).not_to be_valid
expect(build(:direct_upload, :budget_investment, :image, attachment: File.new("spec/fixtures/files/empty.pdf"))).not_to be_valid expect(build(:direct_upload, :budget_investment, :image, attachment: File.new("spec/fixtures/files/empty.pdf"))).not_to be_valid
end end
it "should not be valid without resource_type" do
expect(build(:direct_upload, :proposal, :documents, resource_type: nil)).not_to be_valid
end
it "should not be valid without resource_relation" do
expect(build(:direct_upload, :proposal, :documents, resource_relation: nil)).not_to be_valid
end
it "should not be valid without attachment" do
expect(build(:direct_upload, :proposal, :documents, attachment: nil)).not_to be_valid
end
it "should not be valid without user" do
expect(build(:direct_upload, :proposal, :documents, user: nil)).not_to be_valid
end
end
context "save_attachment" do
it "should create all uploaded file versions" do
proposal_document_direct_upload = build(:direct_upload, :proposal, :documents)
expect{ proposal_document_direct_upload.save_attachment }.to eq(true)
end
end
context "destroy_attachment" do
it "should create all uploaded file versions" do
end
end
end end