Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the #remove_ accessor set the mounted column as changed #1326

Merged

Conversation

nikz
Copy link
Contributor

@nikz nikz commented Feb 27, 2014

This is so that the record is still saved (and the callbacks that do the actual removal are still run) even if no other columns on the model are changed.

This can happen if using accepts_nested_attributes_for with a child model that has a carrierwave mounted column, for instance.

… the remove_ helper so that AR doesn't skip callbacks
@nikz
Copy link
Contributor Author

nikz commented Mar 19, 2014

Hey everyone - we're really keen to get this into mainline so we don't have to keep a fork going including the fix.

Is there anything I can do to help with that? Do I need to pop in a more detailed explanation of the bug? Is the approach I've taken to the fix not what you're looking for?

Thanks!

Nik

bensie added a commit that referenced this pull request Mar 21, 2014
Make the #remove_ accessor set the mounted column as changed
@bensie bensie merged commit 2284a37 into carrierwaveuploader:master Mar 21, 2014
@rmehner
Copy link

rmehner commented Nov 23, 2014

Just ran into the same problem in combination with accepts_nested_attributes_for. This isn't released in an official version yet, right? Is there anything we can do to get this released?

@Awea
Copy link

Awea commented Feb 3, 2015

@rmehner it's seems to not be released yet, I get the same problem. Use the version from github in your Gemfile.

@phlegx
Copy link

phlegx commented Feb 14, 2015

👍 @nikz, @rmehner and @Awea I run in the same problem with accepts_nested_attributes_for! The parameter remove_avatar in nested attributes is submitted, but the avatar still remains. @nikz nice work!

@phlegx
Copy link

phlegx commented Feb 14, 2015

The master branch has this bug solved: https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/orm/activerecord.rb#L72

@nikz file lib/carrierwave/orm/activerecord.rb has changed a lot.

For the moment, using carreirwave v0.10.0 from rubygems I have write a hook as follow:

###
# This hook changes the implementation of carrierwave v0.10.0
#
# 1) Solves remove image using with accepts_nested_attributes_for
# 2) The master branch of carrierwave has solved this issue
#
# config/initializers/hook_carrierwave.rb
module CarrierWave

  module ActiveRecord

    def mount_uploader(column, uploader=nil, options={}, &block)
      super

      alias_method :read_uploader, :read_attribute
      alias_method :write_uploader, :write_attribute
      public :read_uploader
      public :write_uploader

      include CarrierWave::Validations::ActiveModel

      validates_integrity_of column if uploader_option(column.to_sym, :validate_integrity)
      validates_processing_of column if uploader_option(column.to_sym, :validate_processing)
      validates_download_of column if uploader_option(column.to_sym, :validate_download)

      after_save :"store_#{column}!"
      before_save :"write_#{column}_identifier"
      after_commit :"remove_#{column}!", :on => :destroy
      after_commit :"mark_remove_#{column}_false", :on => :update
      before_update :"store_previous_model_for_#{column}"
      after_save :"remove_previously_stored_#{column}"

      class_eval <<-RUBY, __FILE__, __LINE__+1
        def #{column}=(new_file)
          column = _mounter(:#{column}).serialization_column
          send(:"\#{column}_will_change!")
          super
        end

        def remote_#{column}_url=(url)
          column = _mounter(:#{column}).serialization_column
          send(:"\#{column}_will_change!")
          super
        end

        def remove_#{column}=(value)
          send(:"#{column}_will_change!")
          super
        end

        def remove_#{column}!
          super
          _mounter(:#{column}).remove = true
          _mounter(:#{column}).write_identifier
        end

        def serializable_hash(options=nil)
          hash = {}

          except = options && options[:except] && Array.wrap(options[:except]).map(&:to_s)
          only   = options && options[:only]   && Array.wrap(options[:only]).map(&:to_s)

          self.class.uploaders.each do |column, uploader|
            if (!only && !except) || (only && only.include?(column.to_s)) || (!only && except && !except.include?(column.to_s))
              hash[column.to_s] = _mounter(column).uploader.serializable_hash
            end
          end
          super(options).merge(hash)
        end
      RUBY

    end
  end
end

@nikz
Copy link
Contributor Author

nikz commented Feb 14, 2015

@phlegx No problems - if everything is fixed in master maybe we can just close this off?

@phlegx
Copy link

phlegx commented Feb 14, 2015

My opinion, this pull can be closed if a new version, that solves this issue, is released. See my hook. This can help people that continue to use v0.10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants