railsadminteam / rails_admin

RailsAdmin is a Rails engine that provides an easy-to-use interface for managing your data
MIT License
7.9k stars 2.26k forks source link

Filtering/Searching through History Tab Yields Unexpected Results #3064

Open renaehodgkins opened 6 years ago

renaehodgkins commented 6 years ago

I discovered an issue while working on a project for my job. We are using:

When setting up rails_admin_history_rollback with PaperTrail I ran the following migrations:

For the most part everything went flawlessly. However I noticed that trying to filter through results on the "History" tab wasn't bringing back any results. Here's the area I'm talking about specifically, in case it's unclear:

image

When I dug into the rails_admin code to see how the search was supposed to be working, I found this on line 97 of gems/rails_admin-0.8.1/lib/rails_admin/extensions/paper_trail/auditing_adapter.rb:

versions = versions.where('event LIKE ?', "%#{query}%") if query.present?

What that line explained to me was the "filtering" was only searching through the event type. So I went back to my app, searched for "UPDATE" and then I saw search results.

But in my opinion, I'd expect the filter functionality to allow me to search for any of the paper trail version columns.

To that end, I created a module in my own code to override the auditing_adapter code. (I had already done this anyways to add an ip address field to papertrail:

module RailsAdmin
  module Extensions
    module PaperTrail
      class VersionProxy
        def message
           @message = @version.event
        end

        def version_id
          @version.id
        end

        def ip
          @version.ip
        end
      end

      class AuditingAdapter
        protected

        def listing_for_model_or_object(model, object, query, sort, sort_reverse, all, page, per_page)
          if sort.present?
            sort = COLUMN_MAPPING[sort.to_sym]
          else
            sort = :created_at
            sort_reverse = 'true'
          end

          model_name = model.model.name

          current_page = page.presence || '1'

          versions = version_class_for(model_name).where item_type: model_name
          versions = versions.where(item_id: object.id) if object

          # Override to allow proper searching
          if query.present?
            versions = versions.
              joins("INNER JOIN users ON CAST(versions.whodunnit AS INTEGER) = users.id").
              where('object LIKE ? OR object_changes LIKE ? OR users.email ilike ?', "%#{query}%", "%#{query}%", "%#{query}%")
          end

          versions = versions.order(sort_reverse == 'true' ? "#{sort} DESC" : sort)
          versions = all ? versions : versions.send(Kaminari.config.page_method_name, current_page).per(per_page)
          paginated_proxies = Kaminari.paginate_array([], total_count: versions.total_count)
          paginated_proxies = paginated_proxies.page(current_page).per(per_page)
          versions.each do |version|
            paginated_proxies << VersionProxy.new(version, @user_class)
          end
          paginated_proxies
        end
      end
    end
  end
end

The important lines here are:

          # Override to allow proper searching
          if query.present?
            versions = versions.
              joins("INNER JOIN users ON CAST(versions.whodunnit AS INTEGER) = users.id").
              where('object LIKE ? OR object_changes LIKE ? OR users.email ilike ?', "%#{query}%", "%#{query}%", "%#{query}%")
          end

I did the joins on users because whodunnit is a string which holds the user's ID - so I needed to be able to take the whodunnit value, use it as an integer, and look up the users that way. So that if you wanted to filter by the whodunnit/user value, you could do so. But really what I think is probably needed by most people is where('object LIKE ? OR object_changes LIKE ? OR users.email ilike ?', "%#{query}%", "%#{query}%", "%#{query}%")

Would you consider adding something like this so that users can filter through history results?

austinbachman commented 6 years ago

I did a very similar patch a few months ago. It would be great if this would be supported.


module RailsAdmin
  module Extensions
    module PaperTrail
      class AuditingAdapter
      protected
        def listing_for_model_or_object(model, object, query, sort, sort_reverse, all, page, per_page)
          if sort.present?
            sort = COLUMN_MAPPING[sort.to_sym]
          else
            sort = :created_at
            sort_reverse = 'true'
          end

          model_name = model.model.name

          current_page = page.presence || '1'

          versions = version_class_for(model_name).where item_type: model_name
          versions = versions.where item_id: object.id if object
          versions = versions.where('event LIKE ? OR object LIKE ? OR object_changes LIKE ?', "%#{query}%", "%#{query}%", "%#{query}%") if query.present?
          versions = versions.order(sort_reverse == 'true' ? "#{sort} DESC" : sort)
          versions = all ? versions : versions.send(Kaminari.config.page_method_name, current_page).per(per_page)
          paginated_proxies = Kaminari.paginate_array([], total_count: versions.try(:total_count) || versions.count)
          paginated_proxies = paginated_proxies.page(current_page).per(per_page)
          versions.each do |version|
            paginated_proxies << VersionProxy.new(version, @user_class)
          end
          paginated_proxies
        end
      end
    end
  end
end```