hashrocket / decent_exposure

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

An error is raised if :attributes is specified but aren't passed in the request #134

Closed brendon closed 8 years ago

brendon commented 8 years ago

On put, patch, and post, the Strong Parameters strategy blindly calls the :attributes method as defined even though in some cases the request may not have those parameters present (legitimately).

I'm not sure what you think about this. Perhaps I'm wrong in assuming that it would be the case not to have parameters incoming?

I thought about rescuing from the ActionController::ParameterMissing error when trying to call :attributes but I'm not sure that's the best idea. There's no other way to know what the parameters key might be unless we start inferring it again, and then also we'll need to provide a way to override it as in the params strategy.

brendon commented 8 years ago

One solution is to use .fetch instead of .require:

params.fetch(:user, {}).permit(:name, :email, :administrator)

Seems a little bit hacky?

gsobrevilla commented 8 years ago

Hi, I've had this problem sometimes when I wanted to define a custom controller action apart from the typical CRUD actions (index, new, create, edit, update, destroy). For example a POST action disable with the following route:

resources :cars do
 post 'disable', on: :menber
end

in which I would want to do something like:

def disable
  car.enabled = false
  car.save
end

I can't use the car instance fetched by decent_exposure because it raises a ActionController::ParameterMissing, it expects the parameters in the request.

I solve it by fectching mannually the record:

def disable
  self.car = cars.find(params[:id])
  car.enabled = false
  car.save
end

It would be nice to be able to use the instance fetched by decent_exposure in post and put actions that doesn't receive the model parameters, only the id.

Any other solution?

brendon commented 8 years ago

I've got a pull request waiting to be merged: https://github.com/hashrocket/decent_exposure/pull/133 that (if merged) would change what needed to be done to fix this slightly.

The only real fix is to rescue from that error and not assign attributes at all, but that then hides legitimate errors of that kind from reaching the developer. Tricky.

brendon commented 8 years ago

Or we could change the strong parameters strategy to infer the name of the parameters hash unless it has been manually specified. Then we can check for its existence in both cases before calling the filtered parameters (attributes) method. We could default to inferring the name of that method too unless it is specified?

macfanatic commented 8 years ago

Switching from require() to fetch() in the attributes method worked great for me and I'm happy with the solution.

brendon commented 8 years ago

Thanks for the feedback :)

mattpolito commented 8 years ago

Closing post as v3.0 is a different codebase