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

Custom options for reusable params blocks #639

Closed
wants to merge 4 commits into from
Closed

Custom options for reusable params blocks #639

wants to merge 4 commits into from

Conversation

mibon
Copy link
Contributor

@mibon mibon commented May 5, 2014

No description provided.

@dblock
Copy link
Member

dblock commented May 5, 2014

I like it. This needs a CHANGELOG update, please. And I think it could maybe use some elaborating in README. Thx.

@mibon
Copy link
Contributor Author

mibon commented May 5, 2014

Hi. I pushed updates to changelog. Seems like readme already have description of this feature here https://github.com/mibon/grape/commit/4742bbd0fe3a0a2904261c458e45b9dd97d883a2

@dblock
Copy link
Member

dblock commented May 5, 2014

Looked at this more carefully. The spec actually doesn't test that the values of the options are passed through and reused at all. I suggest reverting the test changes you've done so far that conflate two separate goals of the test and adding something like this:

      context 'with block' do
        before do
          subject.helpers do
            params :order do |options|
              optional :order, type: Symbol, values: [:asc, :desc], default: options[:default_order]
              optional :order_by, type: Symbol, values: options[:order_by], default: options[:default_order_by]
            end
          end
        end

        it 'evaluates options' do
          subject.params do
            use :order, default_order: :asc, order_by: [:name, :created_at], default_order_by: :created_at
          end
          expect(subject.settings[:declared_params]).to eq [:order, :order_by]
          # TODO: test here that :order defaults to :asc and that order_by values are :name and :created_at as well as the default being :created_at
        end
      end

@mibon
Copy link
Contributor Author

mibon commented May 6, 2014

I have no idea how to test it. default parameter and possible values are stored as instance variables in validators classes. Of course I can do something like instance_variable_get, but I hope that you can help me with more delicate solution

@dblock
Copy link
Member

dblock commented May 7, 2014

Merged via 943d56b. See my changes in both README and specs. Thanks for contributing!

@dblock dblock closed this May 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants