Skip to content

Handle non-permitted params when comparing rules to request body #1

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/typed_parameters/comparison.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
module TypedParameters
class Comparison
# Example usage:
# Expected types for arguments indicated on the next line.
# Comparison.new(rules_format: RulesFormat, request_body: RequestBody)
#
def initialize(rules_format:, request_body:)
@rules_format, @request_body = rules_format, request_body
end

# Example usage:
# comparison.errors =>
# Returns nil when there are no errors when comparing the
# rules_format to the request_body.
# Otherwise, returns an object where each key is the path to the
# problematic parameter and whose value describes the problem.
def errors
@errors = {}

Expand Down
21 changes: 18 additions & 3 deletions spec/typed_parameters/comparison_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,25 @@
end

context "when the request body does not adhere to the rules format" do
let(:name) { 1_000 }
context "when a non-specified parameter is included in the request body" do
let(:params) { { name: "M@", email: "[email protected]" } }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurko do you think 'extra' params should simply be ignored, or should we actually mark the request as being invalid (this is what we're doing now).

Example:

Rules = { data: Hash }

# the `foo` param is not whitelisted
param = { data: {}, foo: "bar" }

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignored, I think. strong parameters does that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two examples below represent (1) ignoring them, not adding an error but filtering out the non-permitted param, or (2) adding an error as we do now in OTX.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


it "returns an object describing the invalid attribute" do
expect(subject).to eq({ "name" => "param_must_be_string" })
# NOTE: pick one of the following 2 test cases as desired behavior
it "ignores the non-whitelisted param" do
expect(subject).to eq nil
end

it "adds an error for the non-whitelisted param" do
expect(subject).to eq({ "email" => "non_permitted_param" })
end
end

context "when the require parameter is of the wrong type" do
let(:name) { 1_000 }

it "returns an object describing the invalid attribute" do
expect(subject).to eq({ "name" => "param_must_be_string" })
end
end
end
end
Expand Down