hashrocket / decent_exposure

A helper for creating declarative interfaces in controllers
MIT License
1.81k stars 107 forks source link

Using shallow routes #188

Closed brendon closed 6 years ago

brendon commented 6 years ago

Hi there, I was just wondering if there were any established patterns for using decent_exposure 3 with shallow routes in Rails? Sometimes the id will exist and sometimes the exposure will need to be gained from the child resource. How would this look in decent_exposure?

brendon commented 6 years ago

I've hacked around this in my own application and the logic seems to revolve around the existence of the parent resources explicit params[:parent_id] parameter. With shallow routes we can infer that if the parameter exists, then we can load directly with that, otherwise we need to use the child resource to get the parent.

Abstractly it could look like this:

def fetch(scope, id)
  instance = if id
    find(id, scope)
  else
    shallow_child ? shallow_child.send(exposure_name)  : build(build_params, scope)
  end  

  decorate(instance)
end

def id
  shallow_child ? params[:thing_id] : params[:thing_id] || params[:id]
end

def find(id, scope)
  scope.find(id)
end

def build(params, scope)
  scope.new(params) # Thing.new(params)
end

def scope
  model # Thing
end

def model
  exposure_name.classify.constantize # :thing -> Thing
end

def build_params
  if respond_to?(:thing_params, true) && !request.get?
    thing_params
  else
    {}
  end
end

def decorate(thing)
  thing
end

def shallow_child
  nil
end

Implemented something like:

expose :blog, shallow_child: :post
expose :post, from: :blog

The from: on the child would need to modify the scope so that it first checks for params[:blog_id] and if it exists set the scope to blog.posts otherwise Post to prevent an infinite loop.

What do you think?

brendon commented 6 years ago

@mattpolito, I've created a initial PR with my implementation to support shallow routes.

It allows for the following:

expose :blog, shallow_child: :post
expose :posts, from: :blog
expose :post, shallow_parent: :blog

More details in the PR, but it basically allows these two singular methods to depend on each other to find the other. It all swivels around the id existing for the particular resource or not. With shallow routes, there only ever one identifying id. The rest of the exposures need to be inferred from that shallow resource.

I hope you can take a serious look at this, and if you think it's an idea with pursuing, I'd be happy to do tests and refactor it to make it the best it can be :)

mattpolito commented 6 years ago

@brendon Thanks very much for your PR! I love when people are enthused about our project enough to want to contribute.

I understand the issue that you're trying to solve with shallow routes, but it is not a problem we've seen need to be addressed in the years the project has been available. With version 3 of DecentExposure, we've decided to pull back on complex finder features. For the times you are utilizing an exposure that doesn't fit exactly for your method try overriding the exposure in your controller method:


expose(:thing)

def show
   self.thing = Thing.find_another_way
end

Hope that helps

brendon commented 6 years ago

Thanks @mattpolito, I get that I can override the exposures, trouble is, this will be the case for the entire application, so not an exception (when using shallow routes). And because I'd need the parent exposure for every action of the child (accessing scope for permissions checking etc...) I'd then have to move that into a before_action and it just gets messier from there. Overriding it in the exposure was tidier:

expose :faq_area, -> { params[:faq_area_id] ? FaqArea.find(params[:faq_area_id]) : faq.faq_area }
expose :faqs, from: :faq_area
expose :faq, scope: -> { params[:faq_area_id] ? faq_area.faqs : FaqAreaFaq }, model: 'FaqAreaFaq'

becomes the following with the PR:

expose :faq_area, shallow_child: :faq
expose :faqs, from: :faq_area
expose :faq, shallow_parent: :faq_area, model: 'FaqAreaFaq'

I guess I'll try and find a way to abstract the scope and fetch proc's so the exposure definitions aren't so repetitive.

If you change your mind of have any brainwaves on how better to solve this one please let me know :)

mattpolito commented 6 years ago

An exposure config might clean that up

brendon commented 6 years ago

I looked into that, if exposure_config's could accept arguments then definitely. As it stands, there's too much specific to each particular exposure (params id, and relationship method) for an exposure config to be of any use.

Would you be interested in that as a feature? Not sure if it's feasible though, and could end up just looking like an override anyway.

Thanks for thinking further about it :)

brendon commented 6 years ago

I've managed to refactor it down to this:

expose :faq_area, id: :faq_area_id, build: -> { faq.faq_area }
expose :faqs, from: :faq_area
expose :faq, id: :id, model: 'FaqAreaFaq', scope: -> (model) { params[:id] ? model : faq_area.faqs }

I suppose that's not too horrible to look at :)

Just a quick question before I move on with life: is it possible to access the methods of the Behaviour class from the proc's? They're bound to the controller of course, but I'd like to be able to do something like this instead:

expose :faq, id: :id, model: 'FaqAreaFaq', scope: -> (model) { id ? model : faq_area.faqs }

Just a bit shorter.