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

Permit fails in 3.1.1 #929

Closed
AlexVPopov opened this issue May 31, 2016 · 5 comments
Closed

Permit fails in 3.1.1 #929

AlexVPopov opened this issue May 31, 2016 · 5 comments

Comments

@AlexVPopov
Copy link

There have been problems with permit in the past. Currently I encounter the following problem:

Given the specs:

it { should permit(:title, :body).for(:create, params: {note: Fabricate.attributes_for(:note)}).on(:note) }

it { should permit(:title, :body).for(:update, params: {id: note.id, note: {title: 'New title', body: 'New body'}}).on(:note) }

I get the following failure:

 1) V1::NotesController create should (for POST #create) restrict parameters on :note to :title and :body
     Failure/Error: it { should permit(:title, :body).for(:create, params: {note: Fabricate.attributes_for(:note)}).on(:note) }

       Expected POST #create to restrict parameters on :note to :title and :body,
       but it did not restrict any parameters.
     # ./spec/controllers/v1/notes_controller_spec.rb:17:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:45:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:45:in `block (2 levels) in <top (required)>'
     # -e:1:in `<main>'

  2) V1::NotesController update should (for PATCH #update) restrict parameters on :note to :title and :body
     Failure/Error: it { should permit(:title, :body).for(:update, params: {id: note.id, note: {title: 'New title', body: 'New body'}}).on(:note) }

       Expected PATCH #update to restrict parameters on :note to :title and :body,
       but it did not restrict any parameters.
     # ./spec/controllers/v1/notes_controller_spec.rb:69:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:45:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:45:in `block (2 levels) in <top (required)>'
     # -e:1:in `<main>'

Do I use the permit matcher wrong?

@mcmire
Copy link
Collaborator

mcmire commented Jun 14, 2016

Could this be related to #902? In other words, when create or update is called, is params being called twice? It'd be nice to know what your controller looks like.

@AlexVPopov
Copy link
Author

This is the controller:

# frozen_string_literal: true
module V1
  class NotesController < ApplicationController
    rescue_from ActiveRecord::RecordNotFound, with: :render_not_found

    def create
      note = current_user.notes.build(note_params)
      tag_note(note) if tags_param?
      note.save ? render(json: note) : render_validation_failed(note)
    end

    def show
      render json: find_note
    end

    def index
      render json: filter_notes
    end

    def update
      note = find_note
      if note.update(note_params)
        tag_note(note) if tags_param?
        render json: note
      else
        render_validation_failed(note)
      end
    end

    def destroy
      find_note.destroy ? head(:no_content) : render_not_found
    end

    private

      def find_note
        current_user.notes.find(params[:id])
      end

      def note_params
        params.require(:note).permit(:title, :body)
      end

      def render_not_found
        render json: {message: 'Record not found'}, status: :not_found
      end

      def render_validation_failed(note)
        render json: {message: 'Validation failed', errors: note.errors.full_messages},
               status: :unprocessable_entity
      end

      def filter_notes
        current_user.notes
                    .title_matches(params[:title])
                    .body_matches(params[:body])
                    .tag_matches(params[:tag])
                    .any_matches(params[:any])
                    .order(:created_at)
      end

      def tag_note(note)
        current_user.tag(note, with: params[:note][:tags], on: :tags)
      end

      def tags_param?
        params[:note].key?(:tags)
      end
  end
end

If in note_params I put binding.pry

def note_params
  binding.pry
  params.require(:note).permit(:title, :body)
end

the execution doesn't stop, so that means note_params doesn't get called at all and respectively permit doesn't get called.

As for #902 I cannot determine if its related or not, but it possibly is.

@mcmire
Copy link
Collaborator

mcmire commented Jun 9, 2019

Revisiting old bugs and I'm not sure what to do about this, so I'm closing this. I'm happy to reopen this if others are still seeing it, though.

@mcmire mcmire closed this as completed Jun 9, 2019
@pgiblock
Copy link

pgiblock commented Aug 6, 2020

Same issue here. I get

Expected PATCH #update to restrict parameters to :param1, :param2, ...
but it did not restrict any parameters.

With binding.pry() in my def foo_params which is called by the FooController, the execution never halts.

Furthermore, given these two test cases:

      it "permits assignment of permitted attributes" do
        should permit(*@permitted_attrs).for(:update, params: {id: @issue.id})
      end

      it "denies assignment of non-permitted attributes" do
        should_not permit(*@denied_attrs).for(:update, params: {id: @issue.id})
      end

The first one fails and the second one succeeds.

Running shoulda-4.0.0.

@mcmire
Copy link
Collaborator

mcmire commented Aug 6, 2020

@pgiblock Since this is an old issue now, would you mind making a new issue to represent this so we can track it better? Additionally if you wouldn't mind sharing what's inside of your update method that would help me figure out why that test is failing.

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

No branches or pull requests

3 participants