Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Post processing callbacks are run on invalid attachments. #2462

Closed
jhawthorn opened this issue Jul 6, 2017 · 3 comments · Fixed by kreeti/kt-paperclip#16 · May be fixed by #2594
Closed

Post processing callbacks are run on invalid attachments. #2462

jhawthorn opened this issue Jul 6, 2017 · 3 comments · Fixed by kreeti/kt-paperclip#16 · May be fixed by #2594

Comments

@jhawthorn
Copy link
Contributor

From the README

NOTE: Post-processing will not even start if the attachment is not valid according to the validations. Your callbacks and processors will only be called with valid attachments.

However in attachment.rb

instance.run_paperclip_callbacks(:post_process) do
  instance.run_paperclip_callbacks(:"#{name}_post_process") do
    if !@options[:check_validity_before_processing] || !instance.errors.any?
      post_process_styles(*style_args)
    end
  end
end

The check for validity happens inside the run_paperclip_callbacks blocks, so the callbacks are always run.

Reported by @karlentwistle in solidusio/solidus#2064

@karlentwistle
Copy link

karlentwistle commented Jul 6, 2017

I had a go at fixing this as you can see 🙈 I can only get the issue to bubble if I call instance.valid? which does actually return false and would circumvent the post processes for the invalid attachment but this breaks a lot of tests if called here https://github.com/karlentwistle/paperclip/commit/f1fa56b5d1206d80da4fcfc0ffb3bcf468517e7a instead of instance.errors.any?. However instance.errors.any? doesn't contain the relevant error unless instance.valid? is called.

saghaulor added a commit to saghaulor/paperclip that referenced this issue Apr 26, 2018
- Because the processors were called on assignment, instead of during saving,
  the validations could never work correctly. This is because the built in
  validations use the values in the db columns to operate. However, since these
  are populated on assignment, the validations cannot run before the processors
  run. Moreover, any other type of validation not dependent on the db columns
  also cannot run, because the processors are called on assignment. The
  processors should be called during save which allows for validations to occur.

- Fixed tests that assert the incorrect behavior

- Closes thoughtbot#2462, Closes thoughtbot#2321, Closes
  thoughtbot#2236, Closes thoughtbot#2178,
  Closes thoughtbot#1960, Closes thoughtbot#2204
@sidraval
Copy link
Contributor

Looks like there's an open PR that will be merged, closing.

saghaulor added a commit to saghaulor/paperclip that referenced this issue Aug 19, 2018
- Because the processors were called on assignment, instead of during saving,
  the validations could never work correctly. This is because the built in
  validations use the values in the db columns to operate. However, since these
  are populated on assignment, the validations cannot run before the processors
  run. Moreover, any other type of validation not dependent on the db columns
  also cannot run, because the processors are called on assignment. The
  processors should be called during save which allows for validations to occur.

- Fixed tests that assert the incorrect behavior

- Closes thoughtbot#2462, Closes thoughtbot#2321, Closes
  thoughtbot#2236, Closes thoughtbot#2178,
  Closes thoughtbot#1960, Closes thoughtbot#2204
@ssinghi
Copy link

ssinghi commented Dec 30, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.