Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Decorators #1079

Closed
amiel opened this Issue · 6 comments

2 participants

@amiel

Hello,

I use the draper gem a lot and I frequently end up doing this sort of thing:

show do
  # Instead of:
  attributes_table :name, :week_day, :description

  # We must do this:
    panel I18n.t('active_admin.details', :model => active_admin_config.resource_name) do
      attributes_table_for resource.decorator, :name, :week_day, :description
    end
end

I keep on thinking of adding some automatically-use-the-decorator-if-it-is-present code and before I consider making a pull request I'm wondering if other people use decorators in this way and if this feature would be appreciated upstream.

This isn't a request for anyone else to implement this, I'll be happy to do it, I just want to know if there's interest.

@pcreux
Collaborator

Hello,

This is something I'd like to see implemented in ActiveAdmin. It could either auto magically present a decorator to the index / show / form blocks or it could be set in the DSL via: decorator: CustomPostDecorator.

I would love to see you implementing this! :)

@amiel

Ok, I think I have a decent start

https://github.com/carnesmedia/active_admin/compare/1079-decorators

I'm trying to write tests for this, but I haven't been able to get the test suite working. Sometimes I get a stack level too deep and sometimes I get a "[NOTE] You may have encountered a bug in the Ruby interpreter or extension libraries."

@pcreux
Collaborator

@amiel That ight be a bug with your ruby interpreter then...? You can switch rails version by running 'ruby script/use_rails 3.2.0' for instance. Maybe try another ruby? :)

@amiel

@pcreux I'll try more versions of ruby. I did completely re-install ruby-1.9.3-p0 and all gems.
Another issue is that since there are no tests for ActiveAdmin::Views::Pages::Index and ActiveAdmin::Views::Pages::Show, I'm kind of shooting in the dark.
Any help on getting some tests going for those would be appreciated.

@pcreux pcreux was assigned
@amiel

Ok, I got some tests working. I'm not sure exactly what was going on, but the stack-level too deep was my fault, and I think the Ruby interpreter bug was resolved by switching to ruby-1.9.2... shrug

Here's my status on this:

  • Show page uses the decorator if resource.responds_to?(:decorator)
  • Index page uses the decorator if config[:decorator] is given or if it can assume one with "#{ active_admin_config.resource_class_name }Decorator"
  • I have some tests on Show#resource_with_decorator

Still TODO:

  • Write some tests for Index
  • Possibly use the correct const_defined? api to test if the index decorator exists instead of rescuing NameError
  • Document
    • Also note that this requires that FooDecorator.decorate returns a proxy that allows the pagination and scope stuff to still work (recent versions of jcasimir/draper do this, but I'm not sure yet in what version this was introduced).
  • Allow the show page to be configurable the same way as the index page.

@pcreux would it be appropriate to open a pull request now and move the discussion there, or would you rather I wait until all of this is done?

@pcreux
Collaborator

See PR #1117 which is sweet!

@pcreux pcreux closed this
@pcreux pcreux was unassigned by amiel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.